Re: [PATCH v2 05/10] block: introduce BIP_CLONED flag

[Christoph Hellwig <hch@lst.de>]

On Wed, Jun 26, 2024 at 03:36:55PM +0530, Anuj Gupta wrote:
> From: Kanchan Joshi <joshi.k@samsung.com>
> 
> Set the BIP_CLONED flag when bip is cloned.
> Use that flag to ensure that integrity is freed for cloned user bip.
> 
> Note that a bio may have BIO_CLONED flag set but it may still not be
> sharing the integrity vecs.

The design principle of the immutable bio_vecs for the data path
is that BIO_CLONED is just a debug aid and no code should check it.
I'd much prefer to keep that invariant for metadata.

> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 845d4038afb1..8f07c4d0fada 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -147,7 +147,8 @@ void bio_integrity_free(struct bio *bio)
>  	struct bio_integrity_payload *bip = bio_integrity(bio);
>  	struct bio_set *bs = bio->bi_pool;
>  
> -	if (bip->bip_flags & BIP_INTEGRITY_USER)
> +	if (bip->bip_flags & BIP_INTEGRITY_USER &&
> +	    !(bip->bip_flags & BIP_CLONED))
>  		return;
>  	if (bip->bip_flags & BIP_BLOCK_INTEGRITY)
>  		kfree(bvec_virt(bip->bip_vec));

... and the right way to approach this is to clean up the mess
that we have in bio_integrity_free, which probably needs a split up
to deal wit hthe different cases:

 - block layer auto-generated bip_vecs we need it called where it is
   right now, but that side can now unconditionally free the data
   pointed to by the bip_vec
 - for callers that supply PI data themselves, including from user space,
   the caller needs to call __bio_integrity_free and clear
   bi_integrity and REQ_INTEGRITY

this is probably best done by moving the bip_flags checks out of
bio_integrity_free and have bio_integrity_free just do the
unconditional freeing, and have a new helper for
__bio_integrity_endio / bio_integrity_verify_fn to also
free the payload.