On Tue, Jun 22, 2021 at 08:53:09AM +0200, Christoph Hellwig wrote:
> On Wed, Jun 09, 2021 at 08:45:55AM +0800, Ming Lei wrote:
> > index 4d97fb6dd226..bcdff1879c34 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -559,10 +559,14 @@ static inline unsigned int blk_rq_get_max_segments(struct request *rq)
> > static inline int ll_new_hw_segment(struct request *req, struct bio *bio,
> > unsigned int nr_phys_segs)
> > {
> > - if (req->nr_phys_segments + nr_phys_segs > blk_rq_get_max_segments(req))
> > + if (blk_integrity_merge_bio(req->q, req, bio) == false)
> > goto no_merge;
> >
> > - if (blk_integrity_merge_bio(req->q, req, bio) == false)
> > + /* discard request merge won't add new segment */
> > + if (req_op(req) == REQ_OP_DISCARD)
> > + return 1;
> > +
> > + if (req->nr_phys_segments + nr_phys_segs > blk_rq_get_max_segments(req))
>
> I'd rather handle this by returning UINT_MAX for discard requests from
> blk_rq_get_max_segments given that bio_attempt_discard_merge is only used
> for the !DISCARD_MERGE case anyway.
This way makes very big req->nr_phys_segments or overflows it, and may break
drivers.
At least it breaks blk_rq_nr_discard_segments(), which looks easy to fix. But we
need to audit every driver to use req->nr_phys_segments directly in case of
discard request.
Thanks,
Ming
