Re: [PATCH 5/5] dm: implement REQ_FLUSH/FUA support

[Tejun Heo]

Hello,

On 08/17/2010 08:21 PM, Mike Snitzer wrote:
> Why base your work on a partial 2.6.36 tree?  Why not rebase to linus'
> 2.6.36-rc1?

Because the block tree contains enough changes which are not in
mainline yet and bulk of the changes should go through the block tree.

> Once we get the changes in a more suitable state (across the entire
> tree) we can split the individual changes out to their respective
> trees.  Without a comprehensive tree I fear this code isn't going to get
> tested or reviewed properly.
> 
> For example: any review of DM changes, against stale DM code, is wasted
> effort.

Yeap, sure, it will happen all in a good time, but I don't really
agree that review against block tree would be complete waste of
effort.  Things usually don't change that drastically unless dm is
being rewritten as we speak.  Anyways, I'll soon post a rebased /
updated patch.

>> Oh, I was talking about the other way around.  Passing REQ_FUA in
>> bio->bi_rw down to member request_queues.  Sometimes while
>> constructing clone / split bios, the bit is lost (e.g. md raid5).
> 
> Seems we need to change __blk_rq_prep_clone to propagate REQ_FUA just
> like REQ_DISCARD: http://git.kernel.org/linus/3383977

Oooh, right.  I for some reason thought block layer was already doing
that.  I'll update it.  Thanks for pointing it out.

> Doesn't seem like we need to do the same for REQ_FLUSH (at least not for
> rq-based DM's benefit) because dm.c:setup_clone already special cases
> flush requests and sets REQ_FLUSH in cmd_flags.

Hmmm... but in general, I think it would be better to make
__blk_rq_prep_clone() to copy all command related bits.  If some
command bits shouldn't be copied, the caller should take care of them.

Thanks.

-- 
tejun
--
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/