Re: [PATCH, RFC 2/2] dm: support REQ_FLUSH directly

On Fri, Aug 27 2010 at  1:52am -0400,
Jun'ichi Nomura [off-list ref] wrote:

Hi Mike,

(08/27/10 13:08), Mike Snitzer wrote:

quoted

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

No, it's a separate thing.
If we don't need to care about the case where multiple clones
of flush request are necessary, the special casing of flush
request can be removed regardless of the new FLUSH work.

Ah, yes thanks for clarifying.  But we've never cared about multiple
clone of a flush so it's odd that such elaborate infrastructure was
introduced without a need.

quoted

We no longer need waiting now that ordering isn't a concern.  Especially

The waiting is not for ordering, but for multiple clones.

quoted

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.)
         */

This comment is about multiple targets.
The special code for barrier is for single target whose
num_flush_requests > 1. Different thing.

Yes, I need to not send mail just before going to bed..
 

quoted

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

If there will be no need for supporting a request-based target
with num_flush_requests > 1, the special handling of flush
can be removed.

And since there is no such target in the current tree,
I don't object if you remove that part of code for good reason.

OK, certainly something to keep in mind.  But _really_ knowing the
multipath FLUSH+FUA performance difference (extra special-case code vs
none) requires a full FLUSH conversion of request-based DM anyway.

In general, request-based DM's barrier/flush code does carry a certain
maintenance overhead.  It is quite a bit of distracting code in the core
DM which isn't buying us anything.. so we _could_ just remove it and
never look back (until we have some specific need for num_flush_requests

1 in rq-based DM).

Mike