Re: [PATCH 04/10] block: avoid unpinning/freeing the bio_vec incase of cloned bio

Kanchan Joshi
2024-04-29T11:41:08Z

On 4/27/2024 12:35 PM, Christoph Hellwig wrote:
> On Fri, Apr 26, 2024 at 12:09:37AM +0530, Kanchan Joshi wrote:
>> From: Anuj Gupta <anuj20.g@samsung.com>
>>
>> Do it only once when the parent bio completes.
>>
>> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>> ---
>> block/bio-integrity.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
>> index b4042414a08f..b698eb77515d 100644
>> --- a/block/bio-integrity.c
>> +++ b/block/bio-integrity.c
>> @@ -119,7 +119,8 @@ static void bio_integrity_uncopy_user(struct bio_integrity_payload *bip)
>> ret = copy_to_iter(bvec_virt(&src_bvec), bytes, &iter);
>> WARN_ON_ONCE(ret != bytes);
>>
>> - bio_integrity_unpin_bvec(copy, nr_vecs, true);
>> + if (!bio_flagged((bip->bip_bio), BIO_CLONED))
>> + bio_integrity_unpin_bvec(copy, nr_vecs, true);
>> }
>
> This feels wrong. I suspect the problem is that BIP_COPY_USER is
> inherited for clone bios while it shouldn't.
>

But BIP_COPY_USER flag is really required in the clone bio. So that we
can copy the subset of the metadata back (from kernel bounce buffer to
user space pinned buffer) in case of read io.

Overall, copy-back will happen in installments (for each cloned bio),
while the unpin will happen in one shot (for the source bio).