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 <[email protected]> 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
