Re: [PATCH 01/15] block: factor out a bio_integrity_action helper

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 28, 2026 at 05:14:56PM +0100, Christoph Hellwig wrote:
> Split the logic to see if a bio needs integrity metadata from
> bio_integrity_prep into a reusable helper than can be called from
> file system code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Tested-by: Anuj Gupta <anuj20.g@samsung.com>

Looks good to me now, thanks for clarifying the kerneldoc
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  block/bio-integrity-auto.c    | 64 +++++------------------------------
>  block/bio-integrity.c         | 48 ++++++++++++++++++++++++++
>  block/blk-mq.c                |  6 ++--
>  drivers/nvdimm/btt.c          |  6 ++--
>  include/linux/bio-integrity.h |  5 ++-
>  include/linux/blk-integrity.h | 23 +++++++++++++
>  6 files changed, 89 insertions(+), 63 deletions(-)
> 
> diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
> index 44dcdf7520c5..e16f669dbf1e 100644
> --- a/block/bio-integrity-auto.c
> +++ b/block/bio-integrity-auto.c
> @@ -50,11 +50,6 @@ static bool bip_should_check(struct bio_integrity_payload *bip)
>  	return bip->bip_flags & BIP_CHECK_FLAGS;
>  }
>  
> -static bool bi_offload_capable(struct blk_integrity *bi)
> -{
> -	return bi->metadata_size == bi->pi_tuple_size;
> -}
> -
>  /**
>   * __bio_integrity_endio - Integrity I/O completion function
>   * @bio:	Protected bio
> @@ -84,69 +79,27 @@ bool __bio_integrity_endio(struct bio *bio)
>  /**
>   * bio_integrity_prep - Prepare bio for integrity I/O
>   * @bio:	bio to prepare
> + * @action:	preparation action needed (BI_ACT_*)
> + *
> + * Allocate the integrity payload.  For writes, generate the integrity metadata
> + * and for reads, setup the completion handler to verify the metadata.
>   *
> - * Checks if the bio already has an integrity payload attached.  If it does, the
> - * payload has been generated by another kernel subsystem, and we just pass it
> - * through.
> - * Otherwise allocates integrity payload and for writes the integrity metadata
> - * will be generated.  For reads, the completion handler will verify the
> - * metadata.
> + * This is used for bios that do not have user integrity payloads attached.
>   */
> -bool bio_integrity_prep(struct bio *bio)
> +void bio_integrity_prep(struct bio *bio, unsigned int action)
>  {
>  	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
>  	struct bio_integrity_data *bid;
> -	bool set_flags = true;
> -	gfp_t gfp = GFP_NOIO;
> -
> -	if (!bi)
> -		return true;
> -
> -	if (!bio_sectors(bio))
> -		return true;
> -
> -	/* Already protected? */
> -	if (bio_integrity(bio))
> -		return true;
> -
> -	switch (bio_op(bio)) {
> -	case REQ_OP_READ:
> -		if (bi->flags & BLK_INTEGRITY_NOVERIFY) {
> -			if (bi_offload_capable(bi))
> -				return true;
> -			set_flags = false;
> -		}
> -		break;
> -	case REQ_OP_WRITE:
> -		/*
> -		 * Zero the memory allocated to not leak uninitialized kernel
> -		 * memory to disk for non-integrity metadata where nothing else
> -		 * initializes the memory.
> -		 */
> -		if (bi->flags & BLK_INTEGRITY_NOGENERATE) {
> -			if (bi_offload_capable(bi))
> -				return true;
> -			set_flags = false;
> -			gfp |= __GFP_ZERO;
> -		} else if (bi->metadata_size > bi->pi_tuple_size)
> -			gfp |= __GFP_ZERO;
> -		break;
> -	default:
> -		return true;
> -	}
> -
> -	if (WARN_ON_ONCE(bio_has_crypt_ctx(bio)))
> -		return true;
>  
>  	bid = mempool_alloc(&bid_pool, GFP_NOIO);
>  	bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
>  	bid->bio = bio;
>  	bid->bip.bip_flags |= BIP_BLOCK_INTEGRITY;
> -	bio_integrity_alloc_buf(bio, gfp & __GFP_ZERO);
> +	bio_integrity_alloc_buf(bio, action & BI_ACT_ZERO);
>  
>  	bip_set_seed(&bid->bip, bio->bi_iter.bi_sector);
>  
> -	if (set_flags) {
> +	if (action & BI_ACT_CHECK) {
>  		if (bi->csum_type == BLK_INTEGRITY_CSUM_IP)
>  			bid->bip.bip_flags |= BIP_IP_CHECKSUM;
>  		if (bi->csum_type)
> @@ -160,7 +113,6 @@ bool bio_integrity_prep(struct bio *bio)
>  		blk_integrity_generate(bio);
>  	else
>  		bid->saved_bio_iter = bio->bi_iter;
> -	return true;
>  }
>  EXPORT_SYMBOL(bio_integrity_prep);
>  
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 09eeaf6e74b8..6bdbb4ed2d1a 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <linux/blk-integrity.h>
> +#include <linux/t10-pi.h>
>  #include "blk.h"
>  
>  struct bio_integrity_alloc {
> @@ -16,6 +17,53 @@ struct bio_integrity_alloc {
>  
>  static mempool_t integrity_buf_pool;
>  
> +static bool bi_offload_capable(struct blk_integrity *bi)
> +{
> +	return bi->metadata_size == bi->pi_tuple_size;
> +}
> +
> +unsigned int __bio_integrity_action(struct bio *bio)
> +{
> +	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
> +
> +	if (WARN_ON_ONCE(bio_has_crypt_ctx(bio)))
> +		return 0;
> +
> +	switch (bio_op(bio)) {
> +	case REQ_OP_READ:
> +		if (bi->flags & BLK_INTEGRITY_NOVERIFY) {
> +			if (bi_offload_capable(bi))
> +				return 0;
> +			return BI_ACT_BUFFER;
> +		}
> +		return BI_ACT_BUFFER | BI_ACT_CHECK;
> +	case REQ_OP_WRITE:
> +		/*
> +		 * Flush masquerading as write?
> +		 */
> +		if (!bio_sectors(bio))
> +			return 0;
> +
> +		/*
> +		 * Zero the memory allocated to not leak uninitialized kernel
> +		 * memory to disk for non-integrity metadata where nothing else
> +		 * initializes the memory.
> +		 */
> +		if (bi->flags & BLK_INTEGRITY_NOGENERATE) {
> +			if (bi_offload_capable(bi))
> +				return 0;
> +			return BI_ACT_BUFFER | BI_ACT_ZERO;
> +		}
> +
> +		if (bi->metadata_size > bi->pi_tuple_size)
> +			return BI_ACT_BUFFER | BI_ACT_CHECK | BI_ACT_ZERO;
> +		return BI_ACT_BUFFER | BI_ACT_CHECK;
> +	default:
> +		return 0;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(__bio_integrity_action);
> +
>  void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
>  {
>  	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index cf1daedbb39f..d40942bafd02 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3134,6 +3134,7 @@ void blk_mq_submit_bio(struct bio *bio)
>  	struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>  	struct blk_plug *plug = current->plug;
>  	const int is_sync = op_is_sync(bio->bi_opf);
> +	unsigned int integrity_action;
>  	struct blk_mq_hw_ctx *hctx;
>  	unsigned int nr_segs;
>  	struct request *rq;
> @@ -3186,8 +3187,9 @@ void blk_mq_submit_bio(struct bio *bio)
>  	if (!bio)
>  		goto queue_exit;
>  
> -	if (!bio_integrity_prep(bio))
> -		goto queue_exit;
> +	integrity_action = bio_integrity_action(bio);
> +	if (integrity_action)
> +		bio_integrity_prep(bio, integrity_action);
>  
>  	blk_mq_bio_issue_init(q, bio);
>  	if (blk_mq_attempt_bio_merge(q, bio, nr_segs))
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index a933db961ed7..9cc4b659de1a 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -1437,14 +1437,16 @@ static void btt_submit_bio(struct bio *bio)
>  {
>  	struct bio_integrity_payload *bip = bio_integrity(bio);
>  	struct btt *btt = bio->bi_bdev->bd_disk->private_data;
> +	unsigned int integrity_action;
>  	struct bvec_iter iter;
>  	unsigned long start;
>  	struct bio_vec bvec;
>  	int err = 0;
>  	bool do_acct;
>  
> -	if (!bio_integrity_prep(bio))
> -		return;
> +	integrity_action = bio_integrity_action(bio);
> +	if (integrity_action)
> +		bio_integrity_prep(bio, integrity_action);
>  
>  	do_acct = blk_queue_io_stat(bio->bi_bdev->bd_disk->queue);
>  	if (do_acct)
> diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
> index 21e4652dcfd2..276cbbdd2c9d 100644
> --- a/include/linux/bio-integrity.h
> +++ b/include/linux/bio-integrity.h
> @@ -78,7 +78,7 @@ int bio_integrity_add_page(struct bio *bio, struct page *page, unsigned int len,
>  int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter);
>  int bio_integrity_map_iter(struct bio *bio, struct uio_meta *meta);
>  void bio_integrity_unmap_user(struct bio *bio);
> -bool bio_integrity_prep(struct bio *bio);
> +void bio_integrity_prep(struct bio *bio, unsigned int action);
>  void bio_integrity_advance(struct bio *bio, unsigned int bytes_done);
>  void bio_integrity_trim(struct bio *bio);
>  int bio_integrity_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp_mask);
> @@ -104,9 +104,8 @@ static inline void bio_integrity_unmap_user(struct bio *bio)
>  {
>  }
>  
> -static inline bool bio_integrity_prep(struct bio *bio)
> +static inline void bio_integrity_prep(struct bio *bio, unsigned int action)
>  {
> -	return true;
>  }
>  
>  static inline int bio_integrity_clone(struct bio *bio, struct bio *bio_src,
> diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
> index c15b1ac62765..fd3f3c8c0fcd 100644
> --- a/include/linux/blk-integrity.h
> +++ b/include/linux/blk-integrity.h
> @@ -180,4 +180,27 @@ static inline struct bio_vec rq_integrity_vec(struct request *rq)
>  }
>  #endif /* CONFIG_BLK_DEV_INTEGRITY */
>  
> +enum bio_integrity_action {
> +	BI_ACT_BUFFER		= (1u << 0),	/* allocate buffer */
> +	BI_ACT_CHECK		= (1u << 1),	/* generate / verify PI */
> +	BI_ACT_ZERO		= (1u << 2),	/* zero buffer */
> +};
> +
> +/**
> + * bio_integrity_action - return the integrity action needed for a bio
> + * @bio:	bio to operate on
> + *
> + * Returns the mask of integrity actions (BI_ACT_*) that need to be performed
> + * for @bio.
> + */
> +unsigned int __bio_integrity_action(struct bio *bio);
> +static inline unsigned int bio_integrity_action(struct bio *bio)
> +{
> +	if (!blk_get_integrity(bio->bi_bdev->bd_disk))
> +		return 0;
> +	if (bio_integrity(bio))
> +		return 0;
> +	return __bio_integrity_action(bio);
> +}
> +
>  #endif /* _LINUX_BLK_INTEGRITY_H */
> -- 
> 2.47.3
> 
>