Re: [PATCH 1/2] block: fix discard request merge

[Ming Lei <ming.lei@redhat.com>]

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