Re: [PATCH v2] btrfs: raid56: data corruption on a device removal

[Johannes Thumshirn <jthumshirn@suse.de>]

On 07/01/2019 16:34, David Sterba wrote:
> On Mon, Jan 07, 2019 at 12:03:43PM +0100, Johannes Thumshirn wrote:
>>>> + /*
>>>> +  * Since the failed bio can return partial data, bi_sector might be
>>>> +  * incremented by that value. We need to revert it back to the
>>>> +  * state before the bio was submitted.
>>>> +  */
>>>> + physical -= bio->bi_iter.bi_done;
>>>
>>> The bi_done member has been removed in recent block layer changes
>>> commit 7759eb23fd9808a2e4498cf36a798ed65cde78ae ("block: remove
>>> bio_rewind_iter()"). I wonder what kind of block-magic do we need to do
>>> as the iterators seem to be local and there's nothing available in the
>>> call chain leading to find_bio_stripe. Johannes, any ideas?
>>
>> Right, what we could do here is go the same way Ming did in
>> 7759eb23fd980 ("block: remove bio_rewind_iter()") and save a bvec_iter
>> somewhere before submission and then see if we returned partial data,
>> but this somehow feels wrong to me (at least to do in btrfs code instead
>> of the block layer).
> 
>> Ming can we resurrect ->bi_done, or do you have a different suggestion
>> for finding about partial written bios?
> 
> I don't think bi_done can be resurrected
> (https://marc.info/?l=linux-block&m=153549921116441&w=2) but I still am
> not sure that saving the iterator is the right thing (or at least how to
> do that right, not that the whole idea is wrong).

Right, but we're looking for the number of already completed bytes to
rewind here, so from bvec.h's docs it is bi_bvec_done.

Dmitriy can you see if this works for you:

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index e74455eb42f9..2d0e2eec5413 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -1350,6 +1350,7 @@ static int find_bio_stripe(struct btrfs_raid_bio
*rbio,
        struct btrfs_bio_stripe *stripe;

        physical <<= 9;
+       physical -= bio->bi_iter.bi_bvec_done;

        for (i = 0; i < rbio->bbio->num_stripes; i++) {
                stripe = &rbio->bbio->stripes[i];


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850