Re: [PATCH for-next v3 2/2] fs: add ioctl to query metadata and protection info capabilities

[Christoph Hellwig <hch@infradead.org>]

On Tue, Jun 10, 2025 at 06:52:54PM +0530, Anuj Gupta wrote:
> +int blk_get_meta_cap(struct block_device *bdev,
> +		     struct logical_block_metadata_cap __user *argp)
> +{
> +	struct blk_integrity *bi = blk_get_integrity(bdev->bd_disk);
> +	struct logical_block_metadata_cap meta_cap = {};
> +
> +	if (!bi)
> +		goto out;

So is returning an all zeroed structure really the expected interface?
It feels kinda unusual, but if we want to go with that for extensibility
it should probably frow a comment and some language in the man page to
explain what fields to check for support.

> +	if (bi->csum_type == BLK_INTEGRITY_CSUM_NONE) {
> +		/* treat entire tuple as opaque block tag */
> +		meta_cap.lbmd_opaque_size = bi->tuple_size;
> +		goto out;

Purely cosmetic, but the mix of this and the later switch looks a
bit odd.  Instead I'd..

> +	}
> +	meta_cap.lbmd_pi_size = bi->pi_size;
> +	meta_cap.lbmd_pi_offset = bi->pi_offset;
> +	meta_cap.lbmd_opaque_size = bi->tuple_size - bi->pi_size;
> +	if (meta_cap.lbmd_opaque_size && !bi->pi_offset)
> +		meta_cap.lbmd_opaque_offset = bi->pi_size;
> +
> +	meta_cap.lbmd_guard_tag_type = bi->csum_type;

... keep this in the common branch.  All the assignment should do
the right thing even for the non-PI metadata case even if they
do a little extra work.

> +	meta_cap.lbmd_app_tag_size = 2;

And then just guard this with:

	if (bi->csum_type != BLK_INTEGRITY_CSUM_NONE)
		meta_cap.lbmd_app_tag_size = 2;

> +
> +/*
> + * 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

I find this style of comment really hard to read due to the long
lines vs just comments next to the fields.  But it's become
fashionable lately, so..