Re: Block regression since 3.1-rc3

[Mike Snitzer <snitzer@redhat.com>]

On Mon, Oct 10 2011 at  5:33pm -0400,
Tejun Heo <tj@kernel.org> wrote:

> Hello, Mike.
> 
> On Sat, Oct 08, 2011 at 12:14:21PM -0400, Mike Snitzer wrote:
> > Unless others have an immediate ah-ha moment, I'd suggest we revert
> > commit 4853abaae7e4a2a (block: fix flush machinery for stacking drivers
> > with differring flush flags).  Whereby avoiding unnecessarily reentering
> > the flush machinery.
> 
> I don't object to the immediate fix but think that adding such special
> case is gonna make the thing even more brittle and make future changes
> even more difficult.  Those one off cases tend to cause pretty severe
> headache when someone wants to evolve common code, so let's please
> find out what went wrong and fix it properly so that everyone follows
> the same set of rules.

Are you referring to Jeff's fix as "the immediate fix"?  Christophe
seems to have had success with it after all.

As for the special case that you're suggesting makes the code more
brittle, etc.  If you could be more specific that'd be awesome.

Jeff asked a question about the need to kick the queue in this case (as
he didn't feel he had a proper justification for why it was needed).

If we can get a proper patch header together to justify Jeff's patch
that'd be great.  And then revisit any of the special casing you'd like
us to avoid in >= 3.2?

(we're obviously _very_ short on time for a 3.1 fix right now).

> > If commit ed8b752bccf256 (dm table: set flush capability based on
> > underlying devices) is in place the flush gets fed directly to
> > scsi_request_fn, which is fine because the request-based DM's
> > request_queue's flush_flags reflect the flush capabilities of the
> > underlying device(s).
> > 
> > We are then covered relative to the only request-based DM use-case
> > people care about (e.g. dm-multipath, which doesn't use stacked
> > request-based DM).
> > 
> > We can revisit upholding the purity of the flush machinery for stacked
> > devices in >= 3.2.
> 
> Hmmm... another rather nasty assumption the current flush code makes
> is that every flush request has either zero or single bio attached to
> it.  The assumption has always been there for quite some time now.

OK.

> That somehow seems broken by request based dm (either that or wrong
> request is taking INSERT_FLUSH path).

Where was this issue of a flush having multiple bios reported?

> Is it possible that a rq /w single bio can end up with multiple bios
> after cloning?

No, blk_rq_prep_clone() just clones the bio(s) that are attached to the
request being cloned.

Mike