public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Caleb Sander Mateos <csander@purestorage.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	Sagi Grimberg <sagi@grimberg.me>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	Anuj Gupta <anuj20.g@samsung.com>,
	linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
	linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/6] block: fix integrity offset/length conversions
Date: Mon, 20 Apr 2026 22:08:44 -0400	[thread overview]
Message-ID: <yq15x5lqfdx.fsf@ca-mkp.ca.oracle.com> (raw)
In-Reply-To: <CADUfDZrwzUTi2TOj6M-+FtBK6u5evMsWSBqRDwJsLb8yLbOGvw@mail.gmail.com> (Caleb Sander Mateos's message of "Fri, 17 Apr 2026 08:10:35 -0700")


Hi Caleb!

> NVM Command Set specification 1.1 section 5.3.3 requires the reference
> tag to increment by 1 per logical block, so that seems to determine
> the increment unit:

SCSI allows PI to be interleaved at intervals smaller than the logical
block size. This was done for PI compatibility in mixed environments
with both 512[en] and 4Kn disks. Interleaving allows 8 bytes of PI per
512 bytes of data on devices using 4 KB logical blocks. That is the
reason why we use the term "integrity interval" instead of assuming
logical block size.

> The ref tag used for a particular block needs to be consistent. And
> since reftag(block N) can be computed as the reftag(M) + N - M if
> block N is accessed as part of an I/O that begins at block M, the
> function must be of the form reftag(block N) = N + c for some constant
> c. Thus, the ref tag seed needs to be computed in units of logical
> blocks (integrity intervals); no other unit (e.g. 512-byte sectors)
> works.

Whoever attaches the PI decides on the seed value. In the case of the
block layer it made sense to use block layer sector number since that
value is inevitably going to be the same for a future read.

Note that with MD, DM, and partitioning in the mix, the sector number
seen by whoever submits the I/O is going to be different from the LBAs
on the target devices which eventually receive the I/O. Nobody says
there is a computable constant offset. Think scattered LVM extent
allocations. Or RAID stripes placed at mismatched LBA offsets.

> To see the issue with the current approach, consider an example
> accessing LBA 1 on a device with a 4 KB block size. If the block is
> written as part of a write that begins at LBA 0, its ref tag in the
> generated PI will be 1 (sector 0 + 1 integrity interval). If it's
> later read by a read starting at LBA 1, its expected ref tag will be 8
> (sector 8 + 0 integrity intervals), and the auto-integrity code will
> fail the read due to a reftag mismatch.

Something is broken, then. Because the ref tag in the received PI should
have been remapped to start at 8 in that case.

> I agree, the seed doesn't need to match the final LBA, but it does
> need to be in *units* of logical blocks, plus some constant offset.

Your concept of "unit" still sends the wrong message. The seed is an
integer value used to initialize a counter or hardware register. The
seed only has meaning to whichever entity submits the I/O. To everything
else it is a value used for remapping ref tags from the I/O submitter's
point of view to whichever interpretation is mandated by the storage
hardware's PI format.

> With a ublk device. It should affect any block device that supports
> integrity and has a logical block size > 512.

It sounds like the seed value is set incorrectly for reads in your
configuration.

-- 
Martin K. Petersen

      reply	other threads:[~2026-04-21  2:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-17  1:57 [PATCH v3 0/6] block: fix integrity offset/length conversions Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 1/6] block: use integrity interval instead of sector as seed Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 2/6] bio-integrity-fs: pass data iter to bio_integrity_verify() Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 3/6] blk-integrity: take u64 in bio_integrity_intervals() Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 4/6] bio-integrity-fs: use integrity interval instead of sector as seed Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 5/6] t10-pi: use bio_integrity_intervals() helper Caleb Sander Mateos
2026-04-17  1:57 ` [PATCH v3 6/6] blk-integrity: avoid sector_t in bip_{get,set}_seed() Caleb Sander Mateos
2026-04-17  3:26 ` [PATCH v3 0/6] block: fix integrity offset/length conversions Martin K. Petersen
2026-04-17 15:10   ` Caleb Sander Mateos
2026-04-21  2:08     ` Martin K. Petersen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yq15x5lqfdx.fsf@ca-mkp.ca.oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=hch@lst.de \
    --cc=kch@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sagi@grimberg.me \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox