Re: [patch] blk-flush: fix flush policy calculation

[Mike Snitzer]

On Tue, Aug 2, 2011 at 2:17 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Tue, Aug 02, 2011 at 01:39:46PM -0400, Jeff Moyer wrote:

>> So, where do we fix this?  We could just accept Mike's patch to not send
>> such requests down from dm-mpath, but that seems short-sighted.  We
>> could reinstate some checks in __elv_next_request.  Or, we could put the
>> checks into blk_insert_cloned_request.
>>
>> Suggestions?
>
> IMHO, we should fix it at multiple places.
>
> - Your initial fix in blk_insert_flush makes sense. blk_insert_flush()
>  is equivalent of blk_do_flush() so resetting REQ_FLUSH and REQ_FUA there
>  makes sense to me.

Makes sense to me too.

> - Fixing blk_insert_cloned_request() also makes sense to me so that if
>  a request is REQ_FLUSH or REQ_FUA set, we try to add it to underlying
>  device using ELEVATOR_INSERT_FLUSH and not ELEVATOR_INSERT_BACK.

blk_insert_cloned_request is currently only used for request-based DM
(so just dm-mpath).
I don't see a problem with adding logic to it but in practice the DM
change to properly propagate underlying devices' flush_flags is going
to obviate the need (until the kernel grows another
blk_insert_cloned_request caller).

> - Fixing dm-multipath makes sense too as what's the point in dispatching
>  unnecessary flush/fua requests to underlying devices if underlying
>  queue does not have FLUSH capability.

To be clear: It isn't just a fix for dm-multipath.  It fixes all DM
devices (both bio-based and request-based).
That change is already destined for 3.1 via Alasdair's recent DM pull request.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/