Linux block layer
 help / color / mirror / Atom feed
* misc block PI / bounce buffering fixes
@ 2026-09-07  7:40 Christoph Hellwig
  2026-09-07  7:40 ` [PATCH 1/3] block: avoid integer overflows in max_integrity_io_size Christoph Hellwig
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-07  7:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

Hi all,

this series fixes pre-existing issues found by the Sashiko review of
the lazy bounce buffering series.

Diffstat:
 block/bio.c            |   11 +++++++++--
 block/blk-settings.c   |    6 ++++++
 include/linux/blkdev.h |    8 +++++---
 3 files changed, 20 insertions(+), 5 deletions(-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] block: avoid integer overflows in max_integrity_io_size
  2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
@ 2026-09-07  7:40 ` Christoph Hellwig
  2026-09-07  7:40 ` [PATCH 2/3] block: cap atomic write size by PI buffer size constraints Christoph Hellwig
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-07  7:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

Sashiko reports that the calculation in max_integrity_io_size could
overflow a u32 when using 16kiB or larger LBA sizes.  Fix this by
evaluating the maximum size as a 64-bit integer.

Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/blkdev.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4f7905c3412b..098a65f3e48b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1816,9 +1816,11 @@ static inline int bio_split_rw_at(struct bio *bio,
  */
 static inline unsigned int max_integrity_io_size(struct queue_limits *lim)
 {
-	return min_t(unsigned int, lim->max_segment_size,
-		(BLK_INTEGRITY_MAX_SIZE / lim->integrity.metadata_size) <<
-			lim->integrity.interval_exp);
+	u64 max_intervals;
+
+	max_intervals = BLK_INTEGRITY_MAX_SIZE / lim->integrity.metadata_size;
+	return min_t(u64, lim->max_segment_size,
+		max_intervals << lim->integrity.interval_exp);
 }
 
 #define DEFINE_IO_COMP_BATCH(name)	struct io_comp_batch name = { }
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
  2026-09-07  7:40 ` [PATCH 1/3] block: avoid integer overflows in max_integrity_io_size Christoph Hellwig
@ 2026-09-07  7:40 ` Christoph Hellwig
  2026-09-07  8:09   ` John Garry
  2026-09-09  9:16   ` John Garry
  2026-09-07  7:40 ` [PATCH 3/3] block: improve aligning down bios in bio_iov_iter_bounce_write Christoph Hellwig
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-07  7:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

As Sashiko points out, limiting the I/O size by the size of the available
PI buffer can cause inconsistencies for atomic writes.  This also limit
atomic_write_max_sectors in blk_validate_integrity_limits.

Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-settings.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 8274631290db..e469baa1f08b 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -206,6 +206,12 @@ static int blk_validate_integrity_limits(struct queue_limits *lim)
 	lim->max_sectors = min(lim->max_sectors,
 		max_integrity_io_size(lim) >> SECTOR_SHIFT);
 
+	if (lim->features & BLK_FEAT_ATOMIC_WRITES) {
+		lim->atomic_write_max_sectors =
+			min(lim->atomic_write_max_sectors,
+				max_integrity_io_size(lim) >> SECTOR_SHIFT);
+	}
+
 	return 0;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] block: improve aligning down bios in bio_iov_iter_bounce_write
  2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
  2026-09-07  7:40 ` [PATCH 1/3] block: avoid integer overflows in max_integrity_io_size Christoph Hellwig
  2026-09-07  7:40 ` [PATCH 2/3] block: cap atomic write size by PI buffer size constraints Christoph Hellwig
@ 2026-09-07  7:40 ` Christoph Hellwig
  2026-09-09  2:47 ` misc block PI / bounce buffering fixes Martin K. Petersen (Oracle)
  2026-09-11 16:41 ` Jens Axboe
  4 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-07  7:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

Sashiko complained about bio_iov_iter_align_down potentially dropping
bvecs, and thus losing bounce folios.  While losing bio_vecs is real,
bio_iov_iter_bounce_write allocates all segments and thus the entire bio
in multiples of the minsize, thus making the rounding down redundant.

Replace it with a safety rounding down of this_len in case something odd
happens to the greedy folio allocation helper.

Fixes: e7b8b3c5b2a6 ("block: align down bounces bios")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index f95b63c0604a..57ee335899f3 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1389,6 +1389,14 @@ static int bio_iov_iter_bounce_write(struct bio *bio, struct iov_iter *iter,
 		folio = folio_alloc_greedy(GFP_KERNEL, &this_len, minsize);
 		if (!folio)
 			break;
+
+		/*
+		 * Align down the size to the minimum alignment.  In practice
+		 * this should not happen as minsize is expected to be a power
+		 * of two, as is the allocation size, but it offers us a cheap
+		 * extra safety belt.
+		 */
+		this_len &= ~(minsize - 1);
 		bio_add_folio_nofail(bio, folio, this_len, 0);
 
 		if (iter->nofault)
@@ -1416,8 +1424,7 @@ static int bio_iov_iter_bounce_write(struct bio *bio, struct iov_iter *iter,
 
 	if (!bio->bi_iter.bi_size)
 		return -ENOMEM;
-	return bio_iov_iter_align_down(bio, iter,
-			&bio->bi_io_vec[bio->bi_vcnt - 1], minsize - 1);
+	return 0;
 }
 
 static int bio_iov_iter_bounce_read(struct bio *bio, struct iov_iter *iter,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  7:40 ` [PATCH 2/3] block: cap atomic write size by PI buffer size constraints Christoph Hellwig
@ 2026-09-07  8:09   ` John Garry
  2026-09-07  8:46     ` Christoph Hellwig
  2026-09-09  9:16   ` John Garry
  1 sibling, 1 reply; 16+ messages in thread
From: John Garry @ 2026-09-07  8:09 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

On 9/7/26 08:40, Christoph Hellwig wrote:
> As Sashiko points out, 

Do you have a reference for that? I'm just curious, as we don't support 
atomics for integrity writes. Well, for SCSI we don't... we impose this 
restriction via sd_config_atomic(), I need to check for the same in NVMe...

> limiting the I/O size by the size of the available
> PI buffer can cause inconsistencies for atomic writes.  This also limit
> atomic_write_max_sectors in blk_validate_integrity_limits.
> 
> Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/blk-settings.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 8274631290db..e469baa1f08b 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -206,6 +206,12 @@ static int blk_validate_integrity_limits(struct queue_limits *lim)
>   	lim->max_sectors = min(lim->max_sectors,
>   		max_integrity_io_size(lim) >> SECTOR_SHIFT);
>   
> +	if (lim->features & BLK_FEAT_ATOMIC_WRITES) {
> +		lim->atomic_write_max_sectors =
> +			min(lim->atomic_write_max_sectors,
> +				max_integrity_io_size(lim) >> SECTOR_SHIFT);
> +	}
> +
>   	return 0;
>   }
>   


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  8:09   ` John Garry
@ 2026-09-07  8:46     ` Christoph Hellwig
  2026-09-07  9:19       ` John Garry
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-07  8:46 UTC (permalink / raw)
  To: John Garry
  Cc: Christoph Hellwig, Jens Axboe, John Garry, Martin K. Petersen,
	linux-block

On Mon, Sep 07, 2026 at 09:09:47AM +0100, John Garry wrote:
> On 9/7/26 08:40, Christoph Hellwig wrote:
>> As Sashiko points out, 
>
> Do you have a reference for that? I'm just curious, as we don't support 
> atomics for integrity writes. Well, for SCSI we don't... we impose this 
> restriction via sd_config_atomic(), I need to check for the same in NVMe...

https://sashiko.dev/#/patchset/20260831064010.2574896-1-hch%40lst.de

Search for "atomic".


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  8:46     ` Christoph Hellwig
@ 2026-09-07  9:19       ` John Garry
  2026-09-09  2:44         ` Martin K. Petersen
  0 siblings, 1 reply; 16+ messages in thread
From: John Garry @ 2026-09-07  9:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, John Garry, Martin K. Petersen, linux-block

On 9/7/26 09:46, Christoph Hellwig wrote:
> On Mon, Sep 07, 2026 at 09:09:47AM +0100, John Garry wrote:
>> On 9/7/26 08:40, Christoph Hellwig wrote:
>>> As Sashiko points out,
>>
>> Do you have a reference for that? I'm just curious, as we don't support
>> atomics for integrity writes. Well, for SCSI we don't... we impose this
>> restriction via sd_config_atomic(), I need to check for the same in NVMe...
> 
> https://sashiko.dev/#/patchset/20260831064010.2574896-1-hch%40lst.de
> 
> Search for "atomic".

cheers

So this mentions one issue which Tal addresses in "block: fail atomic 
writes instead of falling back to buffered I/O" and how we need to be 
wary of fragmented atomics.

As for atomics on disks with integrity support, we decided not to 
support SCSI WRITE ATOMIC (32) - this one is relevant to PI type 2.

But, overall, I'm not sure on this - should we always disable atomics 
for any disk PI? Is there a use case for the two together? Martin?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  9:19       ` John Garry
@ 2026-09-09  2:44         ` Martin K. Petersen
  2026-09-09  7:52           ` John Garry
  0 siblings, 1 reply; 16+ messages in thread
From: Martin K. Petersen @ 2026-09-09  2:44 UTC (permalink / raw)
  To: John Garry
  Cc: Christoph Hellwig, Jens Axboe, John Garry, Martin K. Petersen,
	linux-block


John,

> As for atomics on disks with integrity support, we decided not to
> support SCSI WRITE ATOMIC (32) - this one is relevant to PI type 2.
>
> But, overall, I'm not sure on this - should we always disable atomics
> for any disk PI? Is there a use case for the two together? Martin?

I think we were originally just trying limit scope. The two features are
orthogonal, I don't see any particular reason why we shouldn't try to
support them concurrently.

Type 2 is fairly rare at this point. But it should be fairly trivial to
add support given the existing READ(32)/WRITE(32) plumbing...

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: misc block PI / bounce buffering fixes
  2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
                   ` (2 preceding siblings ...)
  2026-09-07  7:40 ` [PATCH 3/3] block: improve aligning down bios in bio_iov_iter_bounce_write Christoph Hellwig
@ 2026-09-09  2:47 ` Martin K. Petersen (Oracle)
  2026-09-11 16:41 ` Jens Axboe
  4 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen (Oracle) @ 2026-09-09  2:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, John Garry, Martin K. Petersen, linux-block


Christoph,

> this series fixes pre-existing issues found by the Sashiko review of
> the lazy bounce buffering series.

These look OK to me.

Reviewed-by: Martin K. Petersen <mkp@kernel.org>

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-09  2:44         ` Martin K. Petersen
@ 2026-09-09  7:52           ` John Garry
  0 siblings, 0 replies; 16+ messages in thread
From: John Garry @ 2026-09-09  7:52 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: Christoph Hellwig, Jens Axboe, John Garry, linux-block

On 9/9/26 03:44, Martin K. Petersen wrote:
> 
> John,
> 
>> As for atomics on disks with integrity support, we decided not to
>> support SCSI WRITE ATOMIC (32) - this one is relevant to PI type 2.
>>
>> But, overall, I'm not sure on this - should we always disable atomics
>> for any disk PI? Is there a use case for the two together? Martin?
> 
> I think we were originally just trying limit scope. The two features are
> orthogonal, I don't see any particular reason why we shouldn't try to
> support them concurrently.
> 
> Type 2 is fairly rare at this point. But it should be fairly trivial to
> add support given the existing READ(32)/WRITE(32) plumbing...
> 

Can you please help me understand then how DIF/DIX interacts and atomics 
- specifically, is the userdata and metadata atomically written 
together? I would assume so.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-07  7:40 ` [PATCH 2/3] block: cap atomic write size by PI buffer size constraints Christoph Hellwig
  2026-09-07  8:09   ` John Garry
@ 2026-09-09  9:16   ` John Garry
  2026-09-10  5:36     ` Christoph Hellwig
  1 sibling, 1 reply; 16+ messages in thread
From: John Garry @ 2026-09-09  9:16 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: John Garry, Martin K. Petersen, linux-block

On 9/7/26 08:40, Christoph Hellwig wrote:
> As Sashiko points out, limiting the I/O size by the size of the available
> PI buffer can cause inconsistencies for atomic writes.  This also limit
> atomic_write_max_sectors in blk_validate_integrity_limits.
> 
> Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/blk-settings.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 8274631290db..e469baa1f08b 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -206,6 +206,12 @@ static int blk_validate_integrity_limits(struct queue_limits *lim)
>   	lim->max_sectors = min(lim->max_sectors,
>   		max_integrity_io_size(lim) >> SECTOR_SHIFT);
>   
> +	if (lim->features & BLK_FEAT_ATOMIC_WRITES) {
> +		lim->atomic_write_max_sectors =
> +			min(lim->atomic_write_max_sectors,
> +				max_integrity_io_size(lim) >> SECTOR_SHIFT);
> +	}
> +

A note about the sashiko comment from earlier:
"
The block layer calculates atomic write limits (lim->atomic_write_unit_max)
before blk_validate_integrity_limits() restricts lim->max_sectors. As a
result, the filesystem permits atomic writes up to the unconstrained atomic
limit.
"

atomic writes are not limited by lim->max_sectors, but rather by 
max_hw_sectors. That decision was taken as the atomic limits for a block 
device should be fixed, and not update-able when userspace changes 
max_sectors via sysfs. See this in get_max_io_size():

	if (bio_op(bio) == REQ_OP_WRITE_ZEROES)
		max_sectors = lim->max_write_zeroes_sectors;
	else if (is_atomic)
		max_sectors = lim->atomic_write_max_sectors;
	else
		max_sectors = lim->max_sectors;

Furthermore, I don't think that this is the change above is correct. We 
have 4x atomic limits and I think that 3x would need updating:
- atomic_write_max_sectors
- atomic_write_unit_min
- atomic_write_unit_max

The expectation is that the driver sets atomic_write_hw_max, 
atomic_write_hw_unit_min, and atomic_write_hw_unit_max individually. We 
don't assume that atomic_write_hw_unit_max == 
rounddown_pow_of_two(atomic_write_hw_max). The is because of SCSI and 
it's granularity and alignment atomic limits :(

Anyway, maybe this is a better change:

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 8274631290db..b88744d42719 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -233,11 +233,15 @@ static void blk_atomic_writes_update_limits(struct 
queue_limits *lim)
  	unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT, 
blk_queue_max_guaranteed_bio(lim));

+	unit_limit = min_not_zero(unit_limit, max_integrity_io_size(lim));
  	unit_limit = rounddown_pow_of_two(unit_limit);

  	lim->atomic_write_max_sectors =
  		min(lim->atomic_write_hw_max >> SECTOR_SHIFT,
  			lim->max_hw_sectors);
+	lim->atomic_write_max_sectors =
+		min_not_zero(lim->atomic_write_max_sectors,
+			max_integrity_io_size(lim));
  	lim->atomic_write_unit_min =
  		min(lim->atomic_write_hw_unit_min, unit_limit);
  	lim->atomic_write_unit_max =
@@ -505,11 +509,12 @@ int blk_validate_limits(struct queue_limits *lim)
  	if (!(lim->features & BLK_FEAT_WRITE_CACHE))
  		lim->features &= ~BLK_FEAT_FUA;

-	blk_validate_atomic_write_limits(lim);

  	err = blk_validate_integrity_limits(lim);
  	if (err)
  		return err;
+
+	blk_validate_atomic_write_limits(lim);
  	return blk_validate_zoned_limits(lim);
  }
  EXPORT_SYMBOL_GPL(blk_validate_limits);

Side note:
I would (naively) assume that if the disk has integrity, then it's 
atomic limits reported would update accordingly and 
lim->atomic_write_hw_max et al would already have any integrity limits 
built in (so that we don't need to do any capping of atomic limits by 
integrity limits).

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-09  9:16   ` John Garry
@ 2026-09-10  5:36     ` Christoph Hellwig
  2026-09-10  8:45       ` John Garry
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2026-09-10  5:36 UTC (permalink / raw)
  To: John Garry
  Cc: Christoph Hellwig, Jens Axboe, John Garry, Martin K. Petersen,
	linux-block

On Wed, Sep 09, 2026 at 10:16:17AM +0100, John Garry wrote:
> Furthermore, I don't think that this is the change above is correct. We 
> have 4x atomic limits and I think that 3x would need updating:
> - atomic_write_max_sectors
> - atomic_write_unit_min
> - atomic_write_unit_max
>
> The expectation is that the driver sets atomic_write_hw_max, 
> atomic_write_hw_unit_min, and atomic_write_hw_unit_max individually. We 
> don't assume that atomic_write_hw_unit_max == 
> rounddown_pow_of_two(atomic_write_hw_max). The is because of SCSI and it's 
> granularity and alignment atomic limits :(

>
> Anyway, maybe this is a better change:
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 8274631290db..b88744d42719 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -233,11 +233,15 @@ static void blk_atomic_writes_update_limits(struct 
> queue_limits *lim)
>  	unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT, 
> blk_queue_max_guaranteed_bio(lim));
>
> +	unit_limit = min_not_zero(unit_limit, max_integrity_io_size(lim));
>  	unit_limit = rounddown_pow_of_two(unit_limit);
>
>  	lim->atomic_write_max_sectors =
>  		min(lim->atomic_write_hw_max >> SECTOR_SHIFT,
>  			lim->max_hw_sectors);
> +	lim->atomic_write_max_sectors =
> +		min_not_zero(lim->atomic_write_max_sectors,
> +			max_integrity_io_size(lim));

This needs to be conditional on a non-zero lim->integrity.metadata_size.

Otherwise this looks sane.

> I would (naively) assume that if the disk has integrity, then it's atomic 
> limits reported would update accordingly and lim->atomic_write_hw_max et al 
> would already have any integrity limits built in (so that we don't need to 
> do any capping of atomic limits by integrity limits).

The integrity limit is purely a software limit based on the mempool for the
max integrity data allocation.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-10  5:36     ` Christoph Hellwig
@ 2026-09-10  8:45       ` John Garry
  2026-09-10 12:52         ` John Garry
  0 siblings, 1 reply; 16+ messages in thread
From: John Garry @ 2026-09-10  8:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, John Garry, Martin K. Petersen, linux-block

On 9/10/26 06:36, Christoph Hellwig wrote:
> On Wed, Sep 09, 2026 at 10:16:17AM +0100, John Garry wrote:
>> Furthermore, I don't think that this is the change above is correct. We
>> have 4x atomic limits and I think that 3x would need updating:
>> - atomic_write_max_sectors
>> - atomic_write_unit_min
>> - atomic_write_unit_max
>>
>> The expectation is that the driver sets atomic_write_hw_max,
>> atomic_write_hw_unit_min, and atomic_write_hw_unit_max individually. We
>> don't assume that atomic_write_hw_unit_max ==
>> rounddown_pow_of_two(atomic_write_hw_max). The is because of SCSI and it's
>> granularity and alignment atomic limits 🙁
>> Anyway, maybe this is a better change:
>>
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index 8274631290db..b88744d42719 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -233,11 +233,15 @@ static void blk_atomic_writes_update_limits(struct
>> queue_limits *lim)
>>   	unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT,
>> blk_queue_max_guaranteed_bio(lim));
>>
>> +	unit_limit = min_not_zero(unit_limit, max_integrity_io_size(lim));
>>   	unit_limit = rounddown_pow_of_two(unit_limit);
>>
>>   	lim->atomic_write_max_sectors =
>>   		min(lim->atomic_write_hw_max >> SECTOR_SHIFT,
>>   			lim->max_hw_sectors);
>> +	lim->atomic_write_max_sectors =
>> +		min_not_zero(lim->atomic_write_max_sectors,
>> +			max_integrity_io_size(lim));
> This needs to be conditional on a non-zero lim->integrity.metadata_size.
> 
> Otherwise this looks sane.

ok, I can add that.

I'd like to test this, I suppose scsi_debug is all I have to sanity test 
this...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-10  8:45       ` John Garry
@ 2026-09-10 12:52         ` John Garry
  2026-09-10 14:43           ` John Garry
  0 siblings, 1 reply; 16+ messages in thread
From: John Garry @ 2026-09-10 12:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, John Garry, Martin K. Petersen, linux-block

On 9/10/26 09:45, John Garry wrote:
>>>
>>> The expectation is that the driver sets atomic_write_hw_max,
>>> atomic_write_hw_unit_min, and atomic_write_hw_unit_max individually. We
>>> don't assume that atomic_write_hw_unit_max ==
>>> rounddown_pow_of_two(atomic_write_hw_max). The is because of SCSI and 
>>> it's
>>> granularity and alignment atomic limits 🙁
>>> Anyway, maybe this is a better change:
>>>
>>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>>> index 8274631290db..b88744d42719 100644
>>> --- a/block/blk-settings.c
>>> +++ b/block/blk-settings.c
>>> @@ -233,11 +233,15 @@ static void blk_atomic_writes_update_limits(struct
>>> queue_limits *lim)
>>>       unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT,
>>> blk_queue_max_guaranteed_bio(lim));
>>>
>>> +    unit_limit = min_not_zero(unit_limit, max_integrity_io_size(lim));
>>>       unit_limit = rounddown_pow_of_two(unit_limit);
>>>
>>>       lim->atomic_write_max_sectors =
>>>           min(lim->atomic_write_hw_max >> SECTOR_SHIFT,
>>>               lim->max_hw_sectors);
>>> +    lim->atomic_write_max_sectors =
>>> +        min_not_zero(lim->atomic_write_max_sectors,
>>> +            max_integrity_io_size(lim));
>> This needs to be conditional on a non-zero lim->integrity.metadata_size.
>>
>> Otherwise this looks sane.
> 
> ok, I can add that.
> 
> I'd like to test this, I suppose scsi_debug is all I have to sanity test 
> this...

It's a bit tricky to test this, as max_integrity_io_size() >> 
atomic_write_unit_max normally and so I could not exceed that buffer 
with an atomic write.

However with these changes:

--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1806,7 +1806,7 @@ static inline int bio_split_rw_at(struct bio *bio,
  /*
   * Maximum contiguous integrity buffer allocation.
   */
-#define BLK_INTEGRITY_MAX_SIZE         SZ_2M
+#define BLK_INTEGRITY_MAX_SIZE         SZ_4K

--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -72,7 +72,7 @@ void bio_integrity_alloc_buf(struct bio *bio, gfp_t 
gfp, bool zero_buffer)
         unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
         void *buf;

-       buf = kmalloc(len, gfp | __GFP_NOWARN | (zero_buffer ? 
__GFP_ZERO : 0));
+       buf = NULL;//kmalloc(len, gfp | __GFP_NOWARN | (zero_buffer ? 
__GFP_ZERO : 0));
         if (unlikely(!buf)) {
                 struct page *page;

I can trigger a KASAN use-after-free report:

# cat /sys/block/sda/queue/atomic_write_unit_max_bytes
524288
# xfs_io -d -C "pwrite -b 64k -V 1 -A -D 0 64k" /dev/sda
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 0.0512 sec (1.220 MiB/sec and 19.5164 ops/sec)
# xfs_io -d -C "pwrite -b 512k -V 1 -A -D 0 512k" /dev/sda
[   39.779692] 
==================================================================
[   39.779698] BUG: KASAN: use-after-free in 
blk_integrity_iterate+0x18d5/0x1bb0
[   39.779724] Write of size 2 at addr ffff888118160000 by task xfs_io/244
[   39.779726]
[   39.779729] CPU: 3 UID: 0 PID: 244 Comm: xfs_io Not tainted 
7.3.0-rc2-00042-gb7b6d996bb84-dirty #1238 PREEMPT(lazy)
[   39.779732] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
1.16.3-debian-1.16.3-2 04/01/2014
[   39.779734] Call Trace:
[   39.779736]  <TASK>
[   39.779737]  dump_stack_lvl+0x68/0xa0
[   39.779743]  print_report+0x10d/0x5d0
[   39.779747]  ? __virt_addr_valid+0x21e/0x3f0
[   39.779752]  ? blk_integrity_iterate+0x18d5/0x1bb0
[   39.779754]  kasan_report+0x96/0xd0
[   39.779757]  ? blk_integrity_iterate+0x18d5/0x1bb0
[   39.779760]  blk_integrity_iterate+0x18d5/0x1bb0
[   39.779764]  ? alloc_pages_noprof+0x25/0x70
[   39.779766]  ? mempool_alloc_noprof+0x129/0x1f0
[   39.779770]  ? __pfx_blk_integrity_iterate+0x10/0x10
[   39.779774]  ? bio_integrity_alloc_buf+0x191/0x4a0
[   39.779776]  ? bio_integrity_init+0x25/0xf0
[   39.779779]  blk_mq_submit_bio+0xc21/0x2600
[   39.779784]  ? __pfx_blk_mq_submit_bio+0x10/0x10
[   39.779786]  ? kasan_save_stack+0x34/0x50
[   39.779788]  ? kasan_save_stack+0x24/0x50
[   39.779801]  ? __pfx_iov_iter_extract_pages+0x10/0x10
[   39.779805]  ? __blkdev_direct_IO_simple+0xb5/0x910
[   39.779809]  ? blkdev_write_iter+0x707/0xb80
[   39.779811]  ? do_iter_readv_writev+0x3e4/0x7b0
[   39.779814]  ? __pfx_blk_cgroup_bio_start+0x10/0x10
[   39.779818]  ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[   39.779822]  submit_bio_noacct_nocheck+0x474/0xab0
[   39.779826]  ? __pfx_submit_bio_noacct_nocheck+0x10/0x10
[   39.779829]  ? submit_bio_noacct+0x5bb/0x1330
[   39.779832]  bio_await+0x160/0x190
[   39.779835]  ? __pfx_bio_await+0x10/0x10
[   39.779836]  ? find_held_lock+0x2b/0x80
[   39.779842]  submit_bio_wait+0x11/0x50
[   39.779844]  __blkdev_direct_IO_simple+0x404/0x910
[   39.779847]  ? __pfx___blkdev_direct_IO_simple+0x10/0x10
[   39.779850]  ? __pfx_bio_wait_end_io+0x10/0x10
[   39.779853]  ? inode_set_ctime_current+0x7e/0x610
[   39.779856]  ? inode_set_ctime_to_ts+0xf8/0x330
[   39.779858]  ? __pfx_inode_set_ctime_to_ts+0x10/0x10
[   39.779861]  ? ktime_get_coarse_real_ts64_mg+0x196/0x230
[   39.779864]  ? inode_set_ctime_current+0x7e/0x610
[   39.779867]  blkdev_direct_IO+0xa73/0x1e10
[   39.779870]  ? inode_maybe_inc_iversion+0xe3/0x1a0
[   39.779874]  ? __pfx_inode_maybe_inc_iversion+0x10/0x10
[   39.779876]  ? lock_acquire+0x185/0x2e0
[   39.779880]  ? __pfx_blkdev_direct_IO+0x10/0x10
[   39.779882]  ? __mark_inode_dirty+0x95d/0x10c0
[   39.779886]  ? generic_update_time+0x8c/0xf0
[   39.779888]  blkdev_write_iter+0x707/0xb80
[   39.779892]  do_iter_readv_writev+0x3e4/0x7b0
[   39.779895]  ? __pfx_do_iter_readv_writev+0x10/0x10
[   39.779897]  ? folio_add_lru_vma+0x15b/0x1d0
[   39.779901]  ? selinux_file_permission+0x361/0x4d0
[   39.779905]  vfs_writev+0x269/0xb40
[   39.779907]  ? lock_release+0xc9/0x290
[   39.779911]  ? __pfx_vfs_writev+0x10/0x10
[   39.779913]  ? lock_vma_under_rcu+0x2e1/0x960
[   39.779917]  ? lock_release+0x124/0x290
[   39.779920]  ? lock_vma_under_rcu+0x2eb/0x960
[   39.779925]  ? do_pwritev+0x144/0x220
[   39.779927]  do_pwritev+0x144/0x220
[   39.779929]  ? __pfx_do_pwritev+0x10/0x10
[   39.779931]  ? irqentry_exit+0xf8/0x860
[   39.779934]  ? trace_hardirqs_on_prepare+0x11d/0x150
[   39.779937]  ? irqentry_exit+0x112/0x860
[   39.779940]  do_syscall_64+0x11a/0x610
[   39.779943]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[   39.779946] RIP: 0033:0x7f391f1c9727
[

...

Now I'll fix that patch as suggested and retest.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
  2026-09-10 12:52         ` John Garry
@ 2026-09-10 14:43           ` John Garry
  0 siblings, 0 replies; 16+ messages in thread
From: John Garry @ 2026-09-10 14:43 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, John Garry, Martin K. Petersen, linux-block

On 9/10/26 13:52, John Garry wrote:
> ...
> 
> Now I'll fix that patch as suggested and retest.

Below is the updated change. I suggest that you integrate into this 
series - but I don't mind how it's posted.

--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -240,14 +240,20 @@ static unsigned int 
blk_queue_max_guaranteed_bio(struct queue_limits *lim)

  static void blk_atomic_writes_update_limits(struct queue_limits *lim)
  {
+	unsigned int integrity_max = lim->integrity.metadata_size ?
+					max_integrity_io_size(lim) : 0;
  	unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT,
  					blk_queue_max_guaranteed_bio(lim));

+	unit_limit = min_not_zero(unit_limit, integrity_max);
  	unit_limit = rounddown_pow_of_two(unit_limit);

  	lim->atomic_write_max_sectors =
  		min(lim->atomic_write_hw_max >> SECTOR_SHIFT,
  			lim->max_hw_sectors);
+	lim->atomic_write_max_sectors =
+		min_not_zero(lim->atomic_write_max_sectors,
+				integrity_max >> SECTOR_SHIFT);
  	lim->atomic_write_unit_min =
  		min(lim->atomic_write_hw_unit_min, unit_limit);
  	lim->atomic_write_unit_max =
@@ -515,11 +521,13 @@ int blk_validate_limits(struct queue_limits *lim)
  	if (!(lim->features & BLK_FEAT_WRITE_CACHE))
  		lim->features &= ~BLK_FEAT_FUA;

-	blk_validate_atomic_write_limits(lim);
-
  	err = blk_validate_integrity_limits(lim);
  	if (err)
  		return err;
+
+	/* atomics limits depend on integrity limits */
+	blk_validate_atomic_write_limits(lim);
+
  	return blk_validate_zoned_limits(lim);
  }
  EXPORT_SYMBOL_GPL(blk_validate_limits);

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: misc block PI / bounce buffering fixes
  2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
                   ` (3 preceding siblings ...)
  2026-09-09  2:47 ` misc block PI / bounce buffering fixes Martin K. Petersen (Oracle)
@ 2026-09-11 16:41 ` Jens Axboe
  4 siblings, 0 replies; 16+ messages in thread
From: Jens Axboe @ 2026-09-11 16:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: John Garry, Martin K. Petersen, linux-block


On Mon, 07 Sep 2026 10:40:51 +0300, Christoph Hellwig wrote:
> this series fixes pre-existing issues found by the Sashiko review of
> the lazy bounce buffering series.
> 
> Diffstat:
>  block/bio.c            |   11 +++++++++--
>  block/blk-settings.c   |    6 ++++++
>  include/linux/blkdev.h |    8 +++++---
>  3 files changed, 20 insertions(+), 5 deletions(-)
> 
> [...]

Applied, thanks!

[1/3] block: avoid integer overflows in max_integrity_io_size
      commit: d952f6acb7acc9b94c8c25014b1f468a680c6c00
[2/3] block: cap atomic write size by PI buffer size constraints
      commit: 1418b5633ca973723fd20db5c394e5b031f2bd85
[3/3] block: improve aligning down bios in bio_iov_iter_bounce_write
      commit: 9dd2351aef70512373f79ace94097a32e5ed0f70

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-09-11 16:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07  7:40 misc block PI / bounce buffering fixes Christoph Hellwig
2026-09-07  7:40 ` [PATCH 1/3] block: avoid integer overflows in max_integrity_io_size Christoph Hellwig
2026-09-07  7:40 ` [PATCH 2/3] block: cap atomic write size by PI buffer size constraints Christoph Hellwig
2026-09-07  8:09   ` John Garry
2026-09-07  8:46     ` Christoph Hellwig
2026-09-07  9:19       ` John Garry
2026-09-09  2:44         ` Martin K. Petersen
2026-09-09  7:52           ` John Garry
2026-09-09  9:16   ` John Garry
2026-09-10  5:36     ` Christoph Hellwig
2026-09-10  8:45       ` John Garry
2026-09-10 12:52         ` John Garry
2026-09-10 14:43           ` John Garry
2026-09-07  7:40 ` [PATCH 3/3] block: improve aligning down bios in bio_iov_iter_bounce_write Christoph Hellwig
2026-09-09  2:47 ` misc block PI / bounce buffering fixes Martin K. Petersen (Oracle)
2026-09-11 16:41 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox