diff mbox series

[v4,04/11] block: define meta io descriptor

Message ID 20241016112912.63542-5-anuj20.g@samsung.com (mailing list archive)
State New
Headers show
Series [v4,01/11] block: define set of integrity flags to be inherited by cloned bip | expand

Commit Message

Anuj Gupta Oct. 16, 2024, 11:29 a.m. UTC 
From: Kanchan Joshi <joshi.k@samsung.com>

Introduces a new 'uio_meta' structure that upper layer can
use to pass the meta/integrity information.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 include/linux/bio-integrity.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Keith Busch Oct. 16, 2024, 7:35 p.m. UTC | #1 
On Wed, Oct 16, 2024 at 04:59:05PM +0530, Anuj Gupta wrote:
> +struct uio_meta {
> +	meta_flags_t	flags;
> +	u16		app_tag;
> +	u32		seed;
> +	struct iov_iter iter;
> +};

Is the seed used for anything other than the kernel's t10 generation and
verification? It looks like that's all it is for today, and that part is
skipped for userspace metadata, so I'm not sure we need it.

I know it's been used for passthrough commands since nvme started
supporitng it, but I don't see why the driver ever bothered. I think it
wasn't necessary and we've been carrying it forward ever since.
Anuj Gupta Oct. 17, 2024, 5:49 a.m. UTC | #2 
On 16/10/24 01:35PM, Keith Busch wrote:
>On Wed, Oct 16, 2024 at 04:59:05PM +0530, Anuj Gupta wrote:
>> +struct uio_meta {
>> +	meta_flags_t	flags;
>> +	u16		app_tag;
>> +	u32		seed;
>> +	struct iov_iter iter;
>> +};
>
>Is the seed used for anything other than the kernel's t10 generation and
>verification? It looks like that's all it is for today, and that part is
>skipped for userspace metadata, so I'm not sure we need it.
>
>I know it's been used for passthrough commands since nvme started
>supporitng it, but I don't see why the driver ever bothered. I think it
>wasn't necessary and we've been carrying it forward ever since.

Not for generation/verfication, but seed is used to remap the ref tag when
submitting metadata on a partition (see blk_integrity_prepare/complete).
For cases like partitioning, MD/DM cloning, where virtual start sector is
different from physical sector remapping is required.

It is skipped for passthrough, but we require it for this series where I/O
can be done on partition too. Christoph [1], Martin [2] also expressed
the need for it in the previous version.

[1] https://lore.kernel.org/linux-block/20240824084430.GG8805@lst.de/
[2] https://lore.kernel.org/linux-block/yq17cc0c9p5.fsf@ca-mkp.ca.oracle.com/
Christoph Hellwig Oct. 17, 2024, 7:57 a.m. UTC | #3 
On Wed, Oct 16, 2024 at 04:59:05PM +0530, Anuj Gupta wrote:
> +/* flags for integrity meta */
> +typedef __u16 __bitwise meta_flags_t;
> +
> +struct uio_meta {
> +	meta_flags_t	flags;

Please either add the actual flags here, or if there is a good reason to
do that later add the meta_flags_t type and the member when adding
it.  Also maybe the type name wants a prefix, maybe uio?

Also from looking at the reset of the series uio_meta is in no way
block specific and referenced from io_uring.  So this probably should
go into uio.h?
diff mbox series

Patch

diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 90aab50a3e14..529ec7a8df20 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -30,6 +30,16 @@  struct bio_integrity_payload {
 	struct bio_vec		bip_inline_vecs[];/* embedded bvec array */
 };
 
+/* flags for integrity meta */
+typedef __u16 __bitwise meta_flags_t;
+
+struct uio_meta {
+	meta_flags_t	flags;
+	u16		app_tag;
+	u32		seed;
+	struct iov_iter iter;
+};
+
 #define BIP_CLONE_FLAGS (BIP_MAPPED_INTEGRITY | BIP_CTRL_NOCHECK | \
 			 BIP_DISK_NOCHECK | BIP_IP_CHECKSUM)