On Thu, Aug 26 2010 at  9:43pm -0400,
Jun'ichi Nomura [off-list ref] wrote:

Hi Mike,

(08/27/10 07:50), Mike Snitzer wrote:

quoted

quoted

Special casing is necessary because device-mapper may have to
send multiple copies of REQ_FLUSH request to multiple
targets, while normal request is just sent to single target.

Yes, request-based DM is meant to have all the same capabilities as
bio-based DM.  So in theory it should support multiple targets but in
practice it doesn't.  DM's multipath target is the only consumer of
request-based DM and it only ever clones a single flush request
(num_flush_requests = 1).

This is correct. But,

quoted

So why not remove all of request-based DM's barrier infrastructure and
simply rely on the revised block layer to sequence the FLUSH+WRITE
request for request-based DM?

Given that we do not have a request-based DM target that requires
cloning multiple FLUSH requests its unused code that is delaying DM
support for the new FLUSH+FUA work (NOTE: bio-based DM obviously still
needs work in this area).

the above mentioned 'special casing' is not a hard part.
See the attached patch.

Yes, Tejun suggested something like this in one of the threads.  Thanks
for implementing it.

But do you agree that the request-based barrier code (added in commit
d0bcb8786) could be reverted given the new FLUSH work?

We no longer need waiting now that ordering isn't a concern.  Especially
so given rq-based doesn't support multiple targets.  As you know, from
dm_table_set_type:

        /*
         * Request-based dm supports only tables that have a single target now.
         * To support multiple targets, request splitting support is needed,
         * and that needs lots of changes in the block-layer.
         * (e.g. request completion process for partial completion.)
         */

I think we need to at least benchmark the performance of dm-mpath
without any of this extra, soon to be unnecessary, code.

Maybe my concern is overblown...

The hard part is discerning the error type for flush failure
as discussed in the other thread.
And as Kiyoshi wrote, that's an existing problem so it can
be worked on as a separate issue than the new FLUSH work.

Right, Mike Christie will be refreshing his patchset that should enable
us to resolve that separate issue.

Thanks,
Mike