[PATCH 09/11] block: bypass the STABLE_WRITES flag for protection information

[Hannes Reinecke]

On 6/7/24 07:59, Christoph Hellwig wrote:
> Currently registering a checksum-enabled (aka PI) integrity profile sets
> the QUEUE_FLAG_STABLE_WRITE flag, and unregistering it clears the flag.
> This can incorrectly clear the flag when the driver requires stable
> writes even without PI, e.g. in case of iSCSI or NVMe/TCP with data
> digest enabled.
> 
> Fix this by looking at the csum_type directly in bdev_stable_writes and
> not setting the queue flag.  Also remove the blk_queue_stable_writes
> helper as the only user in nvme wants to only look at the actual
> QUEUE_FLAG_STABLE_WRITE flag as it inherits the integrity configuration
> by other means.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>   block/blk-integrity.c         |  6 ------
>   drivers/nvme/host/multipath.c |  3 ++-
>   include/linux/blkdev.h        | 12 ++++++++----
>   3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index 1d2d371cd632d3..bec0d1df387ce9 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -379,9 +379,6 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
>   	bi->tag_size = template->tag_size;
>   	bi->pi_offset = template->pi_offset;
>   
> -	if (bi->csum_type != BLK_INTEGRITY_CSUM_NONE)
> -		blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
> -
>   #ifdef CONFIG_BLK_INLINE_ENCRYPTION
>   	if (disk->queue->crypto_profile) {
>   		pr_warn("blk-integrity: Integrity and hardware inline encryption are not supported together. Disabling hardware inline encryption.\n");
> @@ -404,9 +401,6 @@ void blk_integrity_unregister(struct gendisk *disk)
>   
>   	if (!bi->tuple_size)
>   		return;
> -
> -	if (bi->csum_type != BLK_INTEGRITY_CSUM_NONE)
> -		blk_queue_flag_clear(QUEUE_FLAG_STABLE_WRITES, disk->queue);
>   	memset(bi, 0, sizeof(*bi));
>   }
>   EXPORT_SYMBOL(blk_integrity_unregister);
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index d8b6b4648eaff9..12c59db02539e5 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -875,7 +875,8 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
>   		nvme_mpath_set_live(ns);
>   	}
>   
> -	if (blk_queue_stable_writes(ns->queue) && ns->head->disk)
> +	if (test_bit(QUEUE_FLAG_STABLE_WRITES, &ns->queue->queue_flags) &&
> +	    ns->head->disk)
>   		blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES,
>   				   ns->head->disk->queue);
>   #ifdef CONFIG_BLK_DEV_ZONED
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index bdd33388e1ced8..f9089750919c6b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -571,8 +571,6 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q);
>   #define blk_queue_noxmerges(q)	\
>   	test_bit(QUEUE_FLAG_NOXMERGES, &(q)->queue_flags)
>   #define blk_queue_nonrot(q)	test_bit(QUEUE_FLAG_NONROT, &(q)->queue_flags)
> -#define blk_queue_stable_writes(q) \
> -	test_bit(QUEUE_FLAG_STABLE_WRITES, &(q)->queue_flags)
>   #define blk_queue_io_stat(q)	test_bit(QUEUE_FLAG_IO_STAT, &(q)->queue_flags)
>   #define blk_queue_add_random(q)	test_bit(QUEUE_FLAG_ADD_RANDOM, &(q)->queue_flags)
>   #define blk_queue_zone_resetall(q)	\
> @@ -1300,8 +1298,14 @@ static inline bool bdev_synchronous(struct block_device *bdev)
>   
>   static inline bool bdev_stable_writes(struct block_device *bdev)
>   {
> -	return test_bit(QUEUE_FLAG_STABLE_WRITES,
> -			&bdev_get_queue(bdev)->queue_flags);
> +	struct request_queue *q = bdev_get_queue(bdev);
> +
> +#ifdef CONFIG_BLK_DEV_INTEGRITY
> +	/* BLK_INTEGRITY_CSUM_NONE is not available in blkdev.h */
> +	if (q->integrity.csum_type != 0)
> +		return true;
> +#endif
> +	return test_bit(QUEUE_FLAG_STABLE_WRITES, &q->queue_flags);
>   }
>   
>   static inline bool bdev_write_cache(struct block_device *bdev)

Reviewed-by: Hannes Reinecke <hare at suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich