Re: dm: clear BIO_SEG_VALID in clone_bio

[yangerkun <yangerkun@huawei.com>]

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
> 
> .
>