[PATCH] dm: clear BIO_SEG_VALID in clone_bio

[PATCH] dm: clear BIO_SEG_VALID in clone_bio

[yangerkun]

Since 57c36519e4("dm: fix clone_bio() to trigger blk_recount_segments()")
has been reverted by fa8db494("dm: don't use bio_trim() afterall"), the
problem that clone bio won't trigger blk_recount_segments will exits
again. So just clean the flag in clone_bio.

Fixes: fa8db4948f522("dm: don't use bio_trim() afterall")
Signed-off-by: yangerkun <yangerkun@huawei.com>
---
 drivers/md/dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 515e6af..b22ac04 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1336,6 +1336,7 @@ static int clone_bio(struct dm_target_io *tio, struct bio *bio,
 			return r;
 	}
 
+	bio_clear_flag(clone, BIO_SEG_VALID);
 	bio_advance(clone, to_bytes(sector - clone->bi_iter.bi_sector));
 	clone->bi_iter.bi_size = to_bytes(len);
 
-- 
2.9.5

Re: dm: clear BIO_SEG_VALID in clone_bio

[Mike Snitzer]

On Thu, Feb 14 2019 at  4:19am -0500,
yangerkun <yangerkun@huawei.com> wrote:

> Since 57c36519e4("dm: fix clone_bio() to trigger blk_recount_segments()")
> has been reverted by fa8db494("dm: don't use bio_trim() afterall"), the
> problem that clone bio won't trigger blk_recount_segments will exits
> again. So just clean the flag in clone_bio.
> 
> Fixes: fa8db4948f522("dm: don't use bio_trim() afterall")
> Signed-off-by: yangerkun <yangerkun@huawei.com>
> ---
>  drivers/md/dm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 515e6af..b22ac04 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1336,6 +1336,7 @@ static int clone_bio(struct dm_target_io *tio, struct bio *bio,
>  			return r;
>  	}
>  
> +	bio_clear_flag(clone, BIO_SEG_VALID);
>  	bio_advance(clone, to_bytes(sector - clone->bi_iter.bi_sector));
>  	clone->bi_iter.bi_size = to_bytes(len);
>  
> -- 
> 2.9.5
> 

Yes, when I wrote commit fa8db4948f522 I purposely did _not_ clear
BIO_SEG_VALID.  Commit 57c36519e4 wasn't born out of an observed need to
blk_recount_segments() (e.g. otherwise something would fail).

So I just reverted to what DM has done in clone_bio() for quite a long
time once using bio_trim() became a liability (since fixed in dm-crypt,
but not staged upstream yet).

Anyway: I'm open to reconsidering, by taking your patch, if there is a
genuine benefit/fix associated with triggering blk_recount_segments()
for clones.  I'm unaware of why it's needed...

Mike

Re: dm: clear BIO_SEG_VALID in clone_bio

[yangerkun]

Hi,

Mike Snitzer wrote on 2019/2/15 2:55:
> On Thu, Feb 14 2019 at  4:19am -0500,
> yangerkun <yangerkun@huawei.com> wrote:
> 
>> Since 57c36519e4("dm: fix clone_bio() to trigger blk_recount_segments()")
>> has been reverted by fa8db494("dm: don't use bio_trim() afterall"), the
>> problem that clone bio won't trigger blk_recount_segments will exits
>> again. So just clean the flag in clone_bio.
>>
>> Fixes: fa8db4948f522("dm: don't use bio_trim() afterall")
>> Signed-off-by: yangerkun <yangerkun@huawei.com>
>> ---
>>   drivers/md/dm.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
>> index 515e6af..b22ac04 100644
>> --- a/drivers/md/dm.c
>> +++ b/drivers/md/dm.c
>> @@ -1336,6 +1336,7 @@ static int clone_bio(struct dm_target_io *tio, struct bio *bio,
>>   			return r;
>>   	}
>>   
>> +	bio_clear_flag(clone, BIO_SEG_VALID);
>>   	bio_advance(clone, to_bytes(sector - clone->bi_iter.bi_sector));
>>   	clone->bi_iter.bi_size = to_bytes(len);
>>   
>> -- 
>> 2.9.5
>>
> 
> Yes, when I wrote commit fa8db4948f522 I purposely did _not_ clear
> BIO_SEG_VALID.  Commit 57c36519e4 wasn't born out of an observed need to
> blk_recount_segments() (e.g. otherwise something would fail).
> 
> So I just reverted to what DM has done in clone_bio() for quite a long
> time once using bio_trim() became a liability (since fixed in dm-crypt,
> but not staged upstream yet).
> 
> Anyway: I'm open to reconsidering, by taking your patch, if there is a
> genuine benefit/fix associated with triggering blk_recount_segments()
> for clones.  I'm unaware of why it's needed...

Re-read the code, clone_bio won't copy flags from source bio, so the 
flag bio_clear_flag of clone won't set anyway. So no need for this 
patch, Right?

Thanks,
Kun.
> 
> Mike
> 
> .
>