On Thu, May 29, 2025 at 05:59:34PM +0000, Eric Biggers wrote:
> On Thu, May 29, 2025 at 12:42:45PM +0530, Anuj Gupta/Anuj Gupta wrote:
> > On 5/29/2025 8:32 AM, Martin K. Petersen wrote:
> > >
> > > Hi Anuj!
> > >
> > > Thanks for working on this!
> > >
> > Hi Martin,
> > Thanks for the feedback!
> >
> > >> 4. tuple_size: size (in bytes) of the protection information tuple.
> > >> 6. pi_offset: offset of protection info within the tuple.
> > >
> > > I find this a little confusing. The T10 PI tuple is <guard, app, ref>.
> > >
> > > I acknowledge things currently are a bit muddy in the block layer since
> > > tuple_size has been transmogrified to hold the NVMe metadata size.
> > >
> > > But for a new user-visible interface I think we should make the
> > > terminology clear. The tuple is the PI and not the rest of the metadata.
> > >
> > > So I think you'd want:
> > >
> > > 4. metadata_size: size (in bytes) of the metadata associated with each interval.
> > > 6. pi_offset: offset of protection information tuple within the metadata.
> > >
> >
> > Yes, this representation looks better. Will make this change.
> >
> > >> +#define FILE_PI_CAP_INTEGRITY (1 << 0)
> > >> +#define FILE_PI_CAP_REFTAG (1 << 1)
> > >
> > > You'll also need to have corresponding uapi defines for:
> > >
> > > enum blk_integrity_checksum {
> > > BLK_INTEGRITY_CSUM_NONE = 0,
> > > BLK_INTEGRITY_CSUM_IP = 1,
> > > BLK_INTEGRITY_CSUM_CRC = 2,
> > > BLK_INTEGRITY_CSUM_CRC64 = 3,
> > > } __packed ;
> > >
> >
> > Right, I'll add these definitions to the UAPI.
>
> Would it make sense to give the CRCs clearer names? For example CRC16_T10DIF
> and CRC64_NVME.
Hm, I wonder whether we should just make all of this an extension of the
new file_getattr() system call we're about to add instead of adding a
separate ioctl for this.
