Re: [PATCH 7/8] block: refactor the bounce buffering code

[Benjamin Block <bblock@linux.ibm.com>]

On Thu, Mar 18, 2021 at 01:53:40PM +0100, Christoph Hellwig wrote:
> On Thu, Mar 18, 2021 at 11:29:50AM +0000, Matthew Wilcox wrote:
> > On Thu, Mar 18, 2021 at 07:39:22AM +0100, Christoph Hellwig wrote:
> > > @@ -536,7 +518,7 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
> > >  					b->max_write_zeroes_sectors);
> > >  	t->max_zone_append_sectors = min(t->max_zone_append_sectors,
> > >  					b->max_zone_append_sectors);
> > > -	t->bounce_pfn = min_not_zero(t->bounce_pfn, b->bounce_pfn);
> > > +	t->bounce = min_not_zero(t->bounce, b->bounce);
> > 
> > I see how min_not_zero() made sense when it was a pfn.  Does it still
> > make sense now it's an enum?  I would have thought it'd now be 'max()',
> > given the definitions later on.
> 
> Actually, blk_stack_limits should not look at ->bounce_pfn / ->bounce
> at all.  blk_queue_bounce is only called my blk_mq_submit_bio, and
> the only stacked blk-mq driver (dm-mpath) does not need bouncing.
> 
> I'll add a patch to fix this up to the front of the series.

Is blk_queue_bounce() called again when mpath submits the request to the
lower device? I thought when I looked at this code some time ago
bouncing would only be checked the first time a request is created
(dm-mpath), and then not again, so when we don't check for whether
bouncing is necessary in mpath, we still might screw the LLD - hence why
we might inherit this via the limits.

-- 
Best Regards, Benjamin Block  / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH    /    https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen         /        Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294