All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Anuj gupta <anuj1072538@gmail.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	Anuj Gupta <anuj20.g@samsung.com>,
	vincent.fu@samsung.com, jack@suse.cz, axboe@kernel.dk,
	viro@zeniv.linux.org.uk, brauner@kernel.org, hch@infradead.org,
	ebiggers@kernel.org, adilger@dilger.ca,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	joshi.k@samsung.com
Subject: Re: [PATCH for-next v2 2/2] fs: add ioctl to query protection info capabilities
Date: Fri, 06 Jun 2025 10:53:08 -0400	[thread overview]
Message-ID: <yq1qzzx9dd2.fsf@ca-mkp.ca.oracle.com> (raw)
In-Reply-To: <CACzX3AujmHHvzBVta2fjrQvytscv5kS0NSgt4iUq-LtXP167BA@mail.gmail.com> (Anuj gupta's message of "Fri, 6 Jun 2025 17:29:02 +0530")


Hi Anuj!

> Thanks for the suggestion. I see your point — especially from the NVMe
> perspective, where integrity buffer can be larger than just the PI
> tuple.
>
> However, since this ioctl is also intended to work on regular files,
> where "metadata" usually refers to inode-level attributes, timestamps,
> permissions, etc., I worry that FS_IOC_METADATA_CAP and
> fs_metadata_cap might be confusing in the broader filesystem context.
>
> Using FS_IOC_GETPI_CAP and struct fs_pi_cap seems more narrowly scoped
> and avoids that ambiguity. Do you see this differently? Or if you have
> a better alternative in mind, I’d be happy to consider it.

I agree it's ambiguous. I just find it equally confusing that "PI" is
now being used to describe opaque metadata which has nothing to do with
PI.

When the block layer code was originally implemented I tried to be
careful about using the term "integrity metadata" everywhere to
distinguish it from "filesystem metadata". It was deliberate not to use
T10 terms in the block layer since there were other competing protection
formats at the time. So one way to get us out of this naming problem
would be to clearly distinguish between for instance "logical block
metadata" and "filesystem metadata".

An alternative would be to revive the concept of "block tagging". The
reason the existing "tag_size" parameter is not called "app_tag_size" is
that the "tag" is "filesystem tag". The fact that it was backed by the
application tag in the PI case was coincidental. The idea at the time
was that filesystems would tag their filesystem blocks with backpointers
and additional information about the contents which would be useful in a
recovery scenario (i.e. "this is a data block for inode XYZ"). So we
could entertain "block tag".

Anyway. I'm thinking something along the lines of this:

* struct logical_block_metadata_cap - Logical block metadata
* @lbmd_flags:			Bitmask of logical block metadata capability flags
* @lbmd_interval:		The amount of data described by each unit of logical block metadata
* @lbmd_size:			Size in bytes of the logical block metadata associated with each interval
* @lbmd_opaque_size:		Size in bytes of the opaque block tag associated with each interval
* @lbmd_opaque_offset:		Offset in bytes of the opaque block tag within the logical block metadata
* @lbmd_pi_size:		Size in bytes of the T10 PI tuple associated with each interval
* @lbmd_pi_offset:		Offset in bytes of T10 PI tuple within the logical block metadata
* @lbmd_pi_guard_tag_type:	T10 PI guard tag type
* @lbmd_pi_app_tag_size:	Size in bytes of the T10 PI application tag
* @lbmd_pi_ref_tag_size:	Size in bytes of the T10 PI reference tag
* @lbmd_pi_storage_tag_size:	Size in bytes of the T10 PI storage tag
* @lbmd_rsvd:			Reserved for future use

That way there's a clear distinction between "all of the metadata", "the
non-PI metadata", and "the PI metadata". Also saves the caller from
doing size and offset calculations by hand for the non-PI stuff.

-- 
Martin K. Petersen

  reply	other threads:[~2025-06-06 14:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20250605150741epcas5p4e5cd0b21137fa714006f44045ff272e2@epcas5p4.samsung.com>
2025-06-05 15:07 ` [PATCH for-next v2 0/2] add ioctl to query protection info capabilities Anuj Gupta
2025-06-05 15:07   ` [PATCH for-next v2 1/2] block: introduce pi_size field in blk_integrity Anuj Gupta
2025-06-06  1:51     ` Martin K. Petersen
2025-06-09  4:10     ` Christoph Hellwig
2025-06-05 15:07   ` [PATCH for-next v2 2/2] fs: add ioctl to query protection info capabilities Anuj Gupta
2025-06-06  2:07     ` Martin K. Petersen
2025-06-06 11:59       ` Anuj gupta
2025-06-06 14:53         ` Martin K. Petersen [this message]
2025-06-09  4:11       ` Christoph Hellwig
2025-06-09  5:44         ` Anuj Gupta/Anuj Gupta
2025-06-10  5:31           ` Christoph Hellwig
2025-06-10 10:26             ` Martin K. Petersen

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=yq1qzzx9dd2.fsf@ca-mkp.ca.oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=adilger@dilger.ca \
    --cc=anuj1072538@gmail.com \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=joshi.k@samsung.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=vincent.fu@samsung.com \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.