Re: [PATCH] block: note about cloned bios and bio_for_each_segment_all

[Jens Axboe <axboe@kernel.dk>]

On 07/14/2017 11:56 AM, Filipe Manana wrote:
> 
> 
> On 07/14/2017 04:03 PM, David Sterba wrote:
>> On Fri, Jul 14, 2017 at 09:47:30PM +0800, Ming Lei wrote:
>>> On Fri, Jul 14, 2017 at 9:40 PM, David Sterba <dsterba@suse.com> wrote:
>>>> We've switched to cloned bios in btrfs and hit a nasty bug leading to
>>>> corruptions, when cloned bios are iterated by bio_for_each_segment_all.
>>>
>>> No, you simply can't use bio_for_each_segment_all on cloned bio, and the
>>> reason is obviously.
>>
>> This was not obvious to us, speaking for the btrfs developers trying to
>> make more use of the of the bio API, so we had to find out the hard way.
> 
> Yep, it might be obvious to those familiar with the block layer's
> internals, but for those not so familiar, it's not. There's no mention
> in bio_clone_fast() that the cloned bio's bi_vcnt shouldn't be used,
> and after finding that, one has to check which bio APIs use it and
> which don't. In this specific btrfs issue, it lead to silent write
> corruptions, making it harder to find (as opposed to crashes or other
> immediate failures).

It's hard to circulate info like that, but the WARN_ON() should have
been there from the get-go. I just need someone to test that patch
triggers for the problematic case, then I'd be happy to get it queued
up.

-- 
Jens Axboe