PI fixes v2

PI fixes v2

[Christoph Hellwig]

Hi all,

this series has two unrelated PI/metadata fixes that came up
during a little testing surge.

Changes since v1:
 - take operator precedence into account so that zeroing doesn't disable
   other GFP_ flags.
 - add a commit log blurb on why Zone Append does not require remapping

Diffstat:
 block/bio-integrity-auto.c    |    2 +-
 block/bio-integrity-fs.c      |    4 ++--
 block/bio-integrity.c         |    9 ++++-----
 include/linux/bio-integrity.h |    2 +-
 4 files changed, 8 insertions(+), 9 deletions(-)

[PATCH 1/2] block: fix GFP_ flags confusion in bio_integrity_alloc_buf

[Christoph Hellwig]

bio_integrity_alloc_buf usage of GFP_ flags is messed up.  For one it
mixes GFP_NOFS and GFP_NOIO for neighbouring allocations, but it also
makes the allocations fail more often than needed.  That code was copied
from bio_alloc_bioset which needs to do that so that it can punt to the
rescuer workqueue, but none of that is needed for the integrity
allocations that either sits in the file system or at the very bottom
of the I/O stack.  Failing early means we'll do a fully waiting
allocation from the mempool ->alloc callback which is usually much
larger than required.

Fix this by passing a gfp_t so that the file system path can pass
GFP_NOFS and the auto-integrity code can pass GFP_NOIO, and don't
modify the allocation type except for disabling warnings.

Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity-auto.c    | 2 +-
 block/bio-integrity-fs.c      | 4 ++--
 block/bio-integrity.c         | 8 +++-----
 include/linux/bio-integrity.h | 2 +-
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
index 353eed632fcc..b1c733ecfd2e 100644
--- a/block/bio-integrity-auto.c
+++ b/block/bio-integrity-auto.c
@@ -94,7 +94,7 @@ void bio_integrity_prep(struct bio *bio, unsigned int action)
 	bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
 	bid->bio = bio;
 	bid->bip.bip_flags |= BIP_BLOCK_INTEGRITY;
-	bio_integrity_alloc_buf(bio, action & BI_ACT_ZERO);
+	bio_integrity_alloc_buf(bio, GFP_NOIO, action & BI_ACT_ZERO);
 	if (action & BI_ACT_CHECK)
 		bio_integrity_setup_default(bio);
 
diff --git a/block/bio-integrity-fs.c b/block/bio-integrity-fs.c
index 0daa42d9ead7..9c5fe5fa8f0d 100644
--- a/block/bio-integrity-fs.c
+++ b/block/bio-integrity-fs.c
@@ -23,10 +23,10 @@ unsigned int fs_bio_integrity_alloc(struct bio *bio)
 	if (!action)
 		return 0;
 
-	iib = mempool_alloc(&fs_bio_integrity_pool, GFP_NOIO);
+	iib = mempool_alloc(&fs_bio_integrity_pool, GFP_NOFS);
 	bio_integrity_init(bio, &iib->bip, &iib->bvec, 1);
 
-	bio_integrity_alloc_buf(bio, action & BI_ACT_ZERO);
+	bio_integrity_alloc_buf(bio, GFP_NOFS, action & BI_ACT_ZERO);
 	if (action & BI_ACT_CHECK)
 		bio_integrity_setup_default(bio);
 	return action;
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e796de1a749e..a53b38cf8a1a 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -64,20 +64,18 @@ unsigned int __bio_integrity_action(struct bio *bio)
 }
 EXPORT_SYMBOL_GPL(__bio_integrity_action);
 
-void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
+void bio_integrity_alloc_buf(struct bio *bio, gfp_t gfp, bool zero_buffer)
 {
 	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
 	struct bio_integrity_payload *bip = bio_integrity(bio);
 	unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
-	gfp_t gfp = GFP_NOIO | (zero_buffer ? __GFP_ZERO : 0);
 	void *buf;
 
-	buf = kmalloc(len, (gfp & ~__GFP_DIRECT_RECLAIM) |
-			__GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN);
+	buf = kmalloc(len, gfp | __GFP_NOWARN | (zero_buffer ? __GFP_ZERO : 0));
 	if (unlikely(!buf)) {
 		struct page *page;
 
-		page = mempool_alloc(&integrity_buf_pool, GFP_NOFS);
+		page = mempool_alloc(&integrity_buf_pool, gfp);
 		if (zero_buffer)
 			memset(page_address(page), 0, len);
 		bvec_set_page(&bip->bip_vec[0], page, len, 0);
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index af5178434ec6..c3dda32fd803 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -141,7 +141,7 @@ static inline int bio_integrity_add_page(struct bio *bio, struct page *page,
 }
 #endif /* CONFIG_BLK_DEV_INTEGRITY */
 
-void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer);
+void bio_integrity_alloc_buf(struct bio *bio, gfp_t gfp, bool zero_buffer);
 void bio_integrity_free_buf(struct bio_integrity_payload *bip);
 void bio_integrity_setup_default(struct bio *bio);
 
-- 
2.53.0

[PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Christoph Hellwig]

Otherwise zone append commands will miss their integrity data.  While
this works "fine" for auto-PI, it break file system PI and non-PI
metadata.

With this XFS on ZNS namespace with non-PI metadata and 512 byte sectors
with PI work, while PI 4k sector formats with PI work only when Caleb's
"block: fix integrity offset/length conversions" is applied as well.

Note that unlike regular writes, zone append does need remapping as
partitions are not supported on zoned block devices.

Fixes: df3c485e0e60 ("block: switch on bio operation in bio_integrity_prep")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index a53b38cf8a1a..b23e2434d80c 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -38,6 +38,7 @@ unsigned int __bio_integrity_action(struct bio *bio)
 		}
 		return BI_ACT_BUFFER | BI_ACT_CHECK;
 	case REQ_OP_WRITE:
+	case REQ_OP_ZONE_APPEND:
 		/*
 		 * Flush masquerading as write?
 		 */
-- 
2.53.0

Re: [PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Caleb Sander Mateos]

On Wed, Jun 24, 2026 at 1:00 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Otherwise zone append commands will miss their integrity data.  While
> this works "fine" for auto-PI, it break file system PI and non-PI
> metadata.
>
> With this XFS on ZNS namespace with non-PI metadata and 512 byte sectors
> with PI work, while PI 4k sector formats with PI work only when Caleb's
> "block: fix integrity offset/length conversions" is applied as well.
>
> Note that unlike regular writes, zone append does need remapping as
> partitions are not supported on zoned block devices.

I take it 4-KB integrity intervals don't work due to the lack of
remapping for REQ_OP_ZONE_APPEND? Sounds like we should come back to
the discussion about cleaning up the ref tag seed and remapping, then.
I never got a reply from Martin on that thread. I guess remapping is
necessary at least for partitioned block devices, but we could skip it
for non-partitioned block devices if we initialized the ref tag seed
correctly.

Best,
Caleb


>
> Fixes: df3c485e0e60 ("block: switch on bio operation in bio_integrity_prep")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bio-integrity.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index a53b38cf8a1a..b23e2434d80c 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -38,6 +38,7 @@ unsigned int __bio_integrity_action(struct bio *bio)
>                 }
>                 return BI_ACT_BUFFER | BI_ACT_CHECK;
>         case REQ_OP_WRITE:
> +       case REQ_OP_ZONE_APPEND:
>                 /*
>                  * Flush masquerading as write?
>                  */
> --
> 2.53.0
>

Re: [PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Christoph Hellwig]

On Wed, Jun 24, 2026 at 08:29:07AM -0700, Caleb Sander Mateos wrote:
> I take it 4-KB integrity intervals don't work due to the lack of
> remapping for REQ_OP_ZONE_APPEND? Sounds like we should come back to
> the discussion about cleaning up the ref tag seed and remapping, then.
> I never got a reply from Martin on that thread. I guess remapping is
> necessary at least for partitioned block devices, but we could skip it
> for non-partitioned block devices if we initialized the ref tag seed
> correctly.

We don't actually need the partition remapping because there can't
be partitions.  But I see on-the-wire reftag value that are 8 times
what they should be, so there is some kind of unit mismatch that your
series fixes.

Re: [PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Caleb Sander Mateos]

On Wed, Jun 24, 2026 at 8:38 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Jun 24, 2026 at 08:29:07AM -0700, Caleb Sander Mateos wrote:
> > I take it 4-KB integrity intervals don't work due to the lack of
> > remapping for REQ_OP_ZONE_APPEND? Sounds like we should come back to
> > the discussion about cleaning up the ref tag seed and remapping, then.
> > I never got a reply from Martin on that thread. I guess remapping is
> > necessary at least for partitioned block devices, but we could skip it
> > for non-partitioned block devices if we initialized the ref tag seed
> > correctly.
>
> We don't actually need the partition remapping because there can't
> be partitions.  But I see on-the-wire reftag value that are 8 times
> what they should be, so there is some kind of unit mismatch that your
> series fixes.

Right, I don't mean partitions of zoned devices, but block devices in
general. I was just trying to understand why the remapping
infrastructure exists in the first place. Seems like we can't remove
it entirely, but we can at least ensure the ref tag seeds are correct
if it's skipped for a non-partitioned device.

Re: [PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Martin K. Petersen]

Caleb,

> Right, I don't mean partitions of zoned devices, but block devices in
> general. I was just trying to understand why the remapping
> infrastructure exists in the first place. Seems like we can't remove
> it entirely, but we can at least ensure the ref tag seeds are correct
> if it's skipped for a non-partitioned device.

The entity attaching the PI to the I/O decides what the seed value
should be.

If you are an application preparing PI, you don't know which LBA a write
is eventually going to end up at. You just know you are writing block 10
inside your file. So you generate PI starting with a reference tag value
of 10 because that is what makes sense to you. And thus you set the seed
to 10 to tell the remapping code which initial reference tag value to
expect in the prepared PI buffer.

Once the write hits the bottom of the stack, we know which initial
reference tag the hardware expects. So we remap the reference tags in
the PI buffer from whatever made sense to the application to whatever
the hardware requires. I.e. an initial value of the lower 32 bits of the
LBA for T10 PI Type 1, incremented by 1 for each subsequent protection
interval.

For reads, it's the same thing. The application wants to read starting
at block offset 10 inside the file so it sets the seed value to 10. At
the bottom of the stack we know how to interpret the PI returned by the
hardware. So we validate that the reference tags received from the
controller match the lower 32 bits of the LBA or whatever the correct PI
format is. And then we remap the reference tags in the received PI
buffer, setting the reference tag to the requested value of 10 for the
first block, and then incrementing by 1 for each subsequent protection
interval.

This allows the application to validate the received reference tags
without ever knowing anything about which start LBA the I/O happened to
come from.

Both DIX and NVMe also allow the hardware to perform the remapping so
the software remapping step can be skipped altogether. Christoph and I
briefly talked about that last week. We currently don't take advantage
of that capability in the NVMe driver.

-- 
Martin K. Petersen

Re: [PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Caleb Sander Mateos]

On Wed, Jun 24, 2026 at 7:29 PM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Caleb,
>
> > Right, I don't mean partitions of zoned devices, but block devices in
> > general. I was just trying to understand why the remapping
> > infrastructure exists in the first place. Seems like we can't remove
> > it entirely, but we can at least ensure the ref tag seeds are correct
> > if it's skipped for a non-partitioned device.
>
> The entity attaching the PI to the I/O decides what the seed value
> should be.
>
> If you are an application preparing PI, you don't know which LBA a write
> is eventually going to end up at. You just know you are writing block 10
> inside your file. So you generate PI starting with a reference tag value
> of 10 because that is what makes sense to you. And thus you set the seed
> to 10 to tell the remapping code which initial reference tag value to
> expect in the prepared PI buffer.

Except currently the ref tag seed is set to sector 80 rather than block 10 :)

>
> Once the write hits the bottom of the stack, we know which initial
> reference tag the hardware expects. So we remap the reference tags in
> the PI buffer from whatever made sense to the application to whatever
> the hardware requires. I.e. an initial value of the lower 32 bits of the
> LBA for T10 PI Type 1, incremented by 1 for each subsequent protection
> interval.

Right, I understand this now. But as Christoph points out,
blk_integrity_prepare() is only called for REQ_OP_WRITE, not
REQ_OP_ZONE_APPEND. I don't think remapping is even possible for zone
appends, as the final LBA isn't known until the device completes the
operation. So I think we need to initialize the ref tag seed in units
of integrity intervals rather than sectors to ensure reading back the
data at an offset uses a ref tag for each block consistent with that
used in the zone append.

>
> For reads, it's the same thing. The application wants to read starting
> at block offset 10 inside the file so it sets the seed value to 10. At
> the bottom of the stack we know how to interpret the PI returned by the
> hardware. So we validate that the reference tags received from the
> controller match the lower 32 bits of the LBA or whatever the correct PI
> format is. And then we remap the reference tags in the received PI
> buffer, setting the reference tag to the requested value of 10 for the
> first block, and then incrementing by 1 for each subsequent protection
> interval.
>
> This allows the application to validate the received reference tags
> without ever knowing anything about which start LBA the I/O happened to
> come from.
>
> Both DIX and NVMe also allow the hardware to perform the remapping so
> the software remapping step can be skipped altogether. Christoph and I
> briefly talked about that last week. We currently don't take advantage
> of that capability in the NVMe driver.

As far as I'm aware, the SCSI driver doesn't use the ref tag seed
either. I see calls to t10_pi_ref_tag() and scsi_prot_ref_tag(), but
bip_get_seed() is only called inside the remapping code. Would indeed
be nice to take advantage of the hardware's ability to accept a ref
tag seed and skip the software remapping. It seems like initializing
the ref tag seed in units of integrity intervals rather than sectors
would be a pre-requisite to that, though. Let me know if I'm
misunderstanding something.

Best,
Caleb

Re: PI fixes v2

[Martin K. Petersen]

Christoph,

> this series has two unrelated PI/metadata fixes that came up
> during a little testing surge.

These look good to me.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen

Re: PI fixes v2

[Jens Axboe]

On Wed, 24 Jun 2026 10:00:01 +0200, Christoph Hellwig wrote:
> this series has two unrelated PI/metadata fixes that came up
> during a little testing surge.
> 
> Changes since v1:
>  - take operator precedence into account so that zeroing doesn't disable
>    other GFP_ flags.
>  - add a commit log blurb on why Zone Append does not require remapping
> 
> [...]

Applied, thanks!

[1/2] block: fix GFP_ flags confusion in bio_integrity_alloc_buf
      commit: e7c1627afda2484baf65449be15873c2550f917a
[2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action
      commit: a1c8bdbbd72564cebb0d02948c1ed57b80b2e773

Best regards,
-- 
Jens Axboe

PI fixes

[Christoph Hellwig]

Hi all,

this series has two unrelated PI/metadata fixes that came up
during a little testing surge.

Diffstat:
 block/bio-integrity-auto.c    |    2 +-
 block/bio-integrity-fs.c      |    4 ++--
 block/bio-integrity.c         |    9 ++++-----
 include/linux/bio-integrity.h |    2 +-
 4 files changed, 8 insertions(+), 9 deletions(-)

[PATCH 2/2] block: handle REQ_OP_ZONE_APPEND in __bio_integrity_action

[Christoph Hellwig]

Otherwise zone append commands will miss their integrity data.  While
this works "fine" for auto-PI, it break file system PI and non-PI
metadata.

With this fix 512-byte block PI and non-PI metadata work fine on native
and emulated Zone Append.  4k block size PI works fine with the
"block: fix integrity offset/length conversions" series from
Caleb Sander Mateos.

Fixes: df3c485e0e60 ("block: switch on bio operation in bio_integrity_prep")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 488eba228c6e..9fb722ace0a8 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -38,6 +38,7 @@ unsigned int __bio_integrity_action(struct bio *bio)
 		}
 		return BI_ACT_BUFFER | BI_ACT_CHECK;
 	case REQ_OP_WRITE:
+	case REQ_OP_ZONE_APPEND:
 		/*
 		 * Flush masquerading as write?
 		 */
-- 
2.53.0