* 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