Linux-Block Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Block integrity with flexibile-offset PI
       [not found] <CGME20240130171918epcas5p3cd0e3e9c7fb9a74c8464b06779c378ea@epcas5p3.samsung.com>
@ 2024-01-30 17:12 ` Kanchan Joshi
       [not found]   ` <CGME20240130171923epcas5p4610296779861b362ef98f3b6f819a060@epcas5p4.samsung.com>
                     ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Kanchan Joshi @ 2024-01-30 17:12 UTC (permalink / raw
  To: kbusch, axboe, hch, martin.petersen, sagi
  Cc: linux-nvme, linux-block, gost.dev, Kanchan Joshi

The block integrity subsystem can only work with PI placed in the first
bytes of the metadata buffer.

The series makes block-integrity support the flexible placement of PI.
And changes NVMe driver to make use of the new capability.

This helps to
(i) enable the more common case for NVMe (PI in last bytes is the norm)
(ii) reduce nop profile users (tried by Jens recently [1]).

/* For NS 4K+16b, 8b PI, last bytes */
Before:
# cat /sys/block/nvme0n1/integrity/format
nop

After:
# cat /sys/block/nvme0n1/integrity/format
T10-DIF-TYPE1-CRC

[1] https://lore.kernel.org/linux-block/20240111160226.1936351-1-axboe@kernel.dk/

Kanchan Joshi (3):
  block: refactor guard helpers
  block: support PI at non-zero offset within metadata
  nvme: allow integrity when PI is not in first bytes

 block/bio-integrity.c         |  1 +
 block/blk-integrity.c         |  1 +
 block/t10-pi.c                | 72 +++++++++++++++++++++++------------
 drivers/nvme/host/core.c      |  8 +++-
 drivers/nvme/host/nvme.h      |  1 +
 include/linux/blk-integrity.h |  1 +
 include/linux/blkdev.h        |  1 +
 7 files changed, 60 insertions(+), 25 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] block: refactor guard helpers
       [not found]   ` <CGME20240130171923epcas5p4610296779861b362ef98f3b6f819a060@epcas5p4.samsung.com>
@ 2024-01-30 17:12     ` Kanchan Joshi
  2024-01-31  7:16       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  0 siblings, 2 replies; 12+ messages in thread
From: Kanchan Joshi @ 2024-01-30 17:12 UTC (permalink / raw
  To: kbusch, axboe, hch, martin.petersen, sagi
  Cc: linux-nvme, linux-block, gost.dev, Kanchan Joshi

Allow computation using the existing guard value.
This is a prep patch.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 block/t10-pi.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/t10-pi.c b/block/t10-pi.c
index 914d8cddd43a..251a7b188963 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -12,14 +12,14 @@
 #include <net/checksum.h>
 #include <asm/unaligned.h>
 
-typedef __be16 (csum_fn) (void *, unsigned int);
+typedef __be16 (csum_fn) (__be16, void *, unsigned int);
 
-static __be16 t10_pi_crc_fn(void *data, unsigned int len)
+static __be16 t10_pi_crc_fn(__be16 crc, void *data, unsigned int len)
 {
-	return cpu_to_be16(crc_t10dif(data, len));
+	return cpu_to_be16(crc_t10dif_update(be16_to_cpu(crc), data, len));
 }
 
-static __be16 t10_pi_ip_fn(void *data, unsigned int len)
+static __be16 t10_pi_ip_fn(__be16 csum, void *data, unsigned int len)
 {
 	return (__force __be16)ip_compute_csum(data, len);
 }
@@ -37,7 +37,7 @@ static blk_status_t t10_pi_generate(struct blk_integrity_iter *iter,
 	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
 		struct t10_pi_tuple *pi = iter->prot_buf;
 
-		pi->guard_tag = fn(iter->data_buf, iter->interval);
+		pi->guard_tag = fn(0, iter->data_buf, iter->interval);
 		pi->app_tag = 0;
 
 		if (type == T10_PI_TYPE1_PROTECTION)
@@ -83,7 +83,7 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
 				goto next;
 		}
 
-		csum = fn(iter->data_buf, iter->interval);
+		csum = fn(0, iter->data_buf, iter->interval);
 
 		if (pi->guard_tag != csum) {
 			pr_err("%s: guard tag error at sector %llu " \
@@ -280,9 +280,9 @@ const struct blk_integrity_profile t10_pi_type3_ip = {
 };
 EXPORT_SYMBOL(t10_pi_type3_ip);
 
-static __be64 ext_pi_crc64(void *data, unsigned int len)
+static __be64 ext_pi_crc64(u64 crc, void *data, unsigned int len)
 {
-	return cpu_to_be64(crc64_rocksoft(data, len));
+	return cpu_to_be64(crc64_rocksoft_update(crc, data, len));
 }
 
 static blk_status_t ext_pi_crc64_generate(struct blk_integrity_iter *iter,
@@ -293,7 +293,7 @@ static blk_status_t ext_pi_crc64_generate(struct blk_integrity_iter *iter,
 	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
 		struct crc64_pi_tuple *pi = iter->prot_buf;
 
-		pi->guard_tag = ext_pi_crc64(iter->data_buf, iter->interval);
+		pi->guard_tag = ext_pi_crc64(0, iter->data_buf, iter->interval);
 		pi->app_tag = 0;
 
 		if (type == T10_PI_TYPE1_PROTECTION)
@@ -343,7 +343,7 @@ static blk_status_t ext_pi_crc64_verify(struct blk_integrity_iter *iter,
 				goto next;
 		}
 
-		csum = ext_pi_crc64(iter->data_buf, iter->interval);
+		csum = ext_pi_crc64(0, iter->data_buf, iter->interval);
 		if (pi->guard_tag != csum) {
 			pr_err("%s: guard tag error at sector %llu " \
 			       "(rcvd %016llx, want %016llx)\n",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] block: support PI at non-zero offset within metadata
       [not found]   ` <CGME20240130171927epcas5p2814181a20402d08b96393ce4a5e06e03@epcas5p2.samsung.com>
@ 2024-01-30 17:12     ` Kanchan Joshi
  2024-01-31  7:21       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  0 siblings, 2 replies; 12+ messages in thread
From: Kanchan Joshi @ 2024-01-30 17:12 UTC (permalink / raw
  To: kbusch, axboe, hch, martin.petersen, sagi
  Cc: linux-nvme, linux-block, gost.dev, Kanchan Joshi, Chinmay Gameti

The block integrity subsystem assumes that PI appears first in the
metadata buffer.
Abolish this assumption by adding the ability to handle PI starting at
a non-zero offset.
Calculation of the guard tag includes the metadata buffer up to this
offset.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
Signed-off-by: Chinmay Gameti <c.gameti@samsung.com>
---
 block/bio-integrity.c         |  1 +
 block/blk-integrity.c         |  1 +
 block/t10-pi.c                | 52 +++++++++++++++++++++++++----------
 include/linux/blk-integrity.h |  1 +
 include/linux/blkdev.h        |  1 +
 5 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c9a16fba58b9..2e3e8e04961e 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -395,6 +395,7 @@ static blk_status_t bio_integrity_process(struct bio *bio,
 	iter.tuple_size = bi->tuple_size;
 	iter.seed = proc_iter->bi_sector;
 	iter.prot_buf = bvec_virt(bip->bip_vec);
+	iter.pi_offset = bi->pi_offset;
 
 	__bio_for_each_segment(bv, bio, bviter, *proc_iter) {
 		void *kaddr = bvec_kmap_local(&bv);
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index d4e9b4556d14..ccbeb6dfa87a 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -370,6 +370,7 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
 	bi->profile = template->profile ? template->profile : &nop_profile;
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;
+	bi->pi_offset = template->pi_offset;
 
 	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
 
diff --git a/block/t10-pi.c b/block/t10-pi.c
index 251a7b188963..80029292ae4c 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -33,11 +33,15 @@ static blk_status_t t10_pi_generate(struct blk_integrity_iter *iter,
 		csum_fn *fn, enum t10_dif_type type)
 {
 	unsigned int i;
+	u8 offset = iter->pi_offset;
 
 	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
-		struct t10_pi_tuple *pi = iter->prot_buf;
+		struct t10_pi_tuple *pi = iter->prot_buf + offset;
 
 		pi->guard_tag = fn(0, iter->data_buf, iter->interval);
+		if (offset)
+			pi->guard_tag = fn(pi->guard_tag, iter->prot_buf,
+					   offset);
 		pi->app_tag = 0;
 
 		if (type == T10_PI_TYPE1_PROTECTION)
@@ -57,11 +61,12 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
 		csum_fn *fn, enum t10_dif_type type)
 {
 	unsigned int i;
+	u8 offset = iter->pi_offset;
 
 	BUG_ON(type == T10_PI_TYPE0_PROTECTION);
 
 	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
-		struct t10_pi_tuple *pi = iter->prot_buf;
+		struct t10_pi_tuple *pi = iter->prot_buf + offset;
 		__be16 csum;
 
 		if (type == T10_PI_TYPE1_PROTECTION ||
@@ -84,6 +89,8 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
 		}
 
 		csum = fn(0, iter->data_buf, iter->interval);
+		if (offset)
+			csum = fn(csum, iter->prot_buf, offset);
 
 		if (pi->guard_tag != csum) {
 			pr_err("%s: guard tag error at sector %llu " \
@@ -134,9 +141,11 @@ static blk_status_t t10_pi_type1_verify_ip(struct blk_integrity_iter *iter)
  */
 static void t10_pi_type1_prepare(struct request *rq)
 {
-	const int tuple_sz = rq->q->integrity.tuple_size;
+	struct blk_integrity *bi = &rq->q->integrity;
+	const int tuple_sz = bi->tuple_size;
 	u32 ref_tag = t10_pi_ref_tag(rq);
 	struct bio *bio;
+	u8 offset = bi->pi_offset;
 
 	__rq_for_each_bio(bio, rq) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
@@ -154,7 +163,7 @@ static void t10_pi_type1_prepare(struct request *rq)
 
 			p = bvec_kmap_local(&iv);
 			for (j = 0; j < iv.bv_len; j += tuple_sz) {
-				struct t10_pi_tuple *pi = p;
+				struct t10_pi_tuple *pi = p + offset;
 
 				if (be32_to_cpu(pi->ref_tag) == virt)
 					pi->ref_tag = cpu_to_be32(ref_tag);
@@ -183,10 +192,12 @@ static void t10_pi_type1_prepare(struct request *rq)
  */
 static void t10_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
 {
-	unsigned intervals = nr_bytes >> rq->q->integrity.interval_exp;
-	const int tuple_sz = rq->q->integrity.tuple_size;
+	struct blk_integrity *bi = &rq->q->integrity;
+	unsigned intervals = nr_bytes >> bi->interval_exp;
+	const int tuple_sz = bi->tuple_size;
 	u32 ref_tag = t10_pi_ref_tag(rq);
 	struct bio *bio;
+	u8 offset = bi->pi_offset;
 
 	__rq_for_each_bio(bio, rq) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
@@ -200,7 +211,7 @@ static void t10_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
 
 			p = bvec_kmap_local(&iv);
 			for (j = 0; j < iv.bv_len && intervals; j += tuple_sz) {
-				struct t10_pi_tuple *pi = p;
+				struct t10_pi_tuple *pi = p + offset;
 
 				if (be32_to_cpu(pi->ref_tag) == ref_tag)
 					pi->ref_tag = cpu_to_be32(virt);
@@ -289,11 +300,15 @@ static blk_status_t ext_pi_crc64_generate(struct blk_integrity_iter *iter,
 					enum t10_dif_type type)
 {
 	unsigned int i;
+	u8 offset = iter->pi_offset;
 
 	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
-		struct crc64_pi_tuple *pi = iter->prot_buf;
+		struct crc64_pi_tuple *pi = iter->prot_buf + offset;
 
 		pi->guard_tag = ext_pi_crc64(0, iter->data_buf, iter->interval);
+		if (offset)
+			pi->guard_tag = ext_pi_crc64(be64_to_cpu(pi->guard_tag),
+					iter->prot_buf, offset);
 		pi->app_tag = 0;
 
 		if (type == T10_PI_TYPE1_PROTECTION)
@@ -320,9 +335,10 @@ static blk_status_t ext_pi_crc64_verify(struct blk_integrity_iter *iter,
 				      enum t10_dif_type type)
 {
 	unsigned int i;
+	u8 offset = iter->pi_offset;
 
 	for (i = 0; i < iter->data_size; i += iter->interval) {
-		struct crc64_pi_tuple *pi = iter->prot_buf;
+		struct crc64_pi_tuple *pi = iter->prot_buf + offset;
 		u64 ref, seed;
 		__be64 csum;
 
@@ -344,6 +360,10 @@ static blk_status_t ext_pi_crc64_verify(struct blk_integrity_iter *iter,
 		}
 
 		csum = ext_pi_crc64(0, iter->data_buf, iter->interval);
+		if (offset)
+			csum = ext_pi_crc64(be64_to_cpu(csum), iter->prot_buf,
+					    offset);
+
 		if (pi->guard_tag != csum) {
 			pr_err("%s: guard tag error at sector %llu " \
 			       "(rcvd %016llx, want %016llx)\n",
@@ -373,9 +393,11 @@ static blk_status_t ext_pi_type1_generate_crc64(struct blk_integrity_iter *iter)
 
 static void ext_pi_type1_prepare(struct request *rq)
 {
-	const int tuple_sz = rq->q->integrity.tuple_size;
+	struct blk_integrity *bi = &rq->q->integrity;
+	const int tuple_sz = bi->tuple_size;
 	u64 ref_tag = ext_pi_ref_tag(rq);
 	struct bio *bio;
+	u8 offset = bi->pi_offset;
 
 	__rq_for_each_bio(bio, rq) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
@@ -393,7 +415,7 @@ static void ext_pi_type1_prepare(struct request *rq)
 
 			p = bvec_kmap_local(&iv);
 			for (j = 0; j < iv.bv_len; j += tuple_sz) {
-				struct crc64_pi_tuple *pi = p;
+				struct crc64_pi_tuple *pi = p +  offset;
 				u64 ref = get_unaligned_be48(pi->ref_tag);
 
 				if (ref == virt)
@@ -411,10 +433,12 @@ static void ext_pi_type1_prepare(struct request *rq)
 
 static void ext_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
 {
-	unsigned intervals = nr_bytes >> rq->q->integrity.interval_exp;
-	const int tuple_sz = rq->q->integrity.tuple_size;
+	struct blk_integrity *bi = &rq->q->integrity;
+	unsigned intervals = nr_bytes >> bi->interval_exp;
+	const int tuple_sz = bi->tuple_size;
 	u64 ref_tag = ext_pi_ref_tag(rq);
 	struct bio *bio;
+	u8 offset = bi->pi_offset;
 
 	__rq_for_each_bio(bio, rq) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
@@ -428,7 +452,7 @@ static void ext_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
 
 			p = bvec_kmap_local(&iv);
 			for (j = 0; j < iv.bv_len && intervals; j += tuple_sz) {
-				struct crc64_pi_tuple *pi = p;
+				struct crc64_pi_tuple *pi = p + offset;
 				u64 ref = get_unaligned_be48(pi->ref_tag);
 
 				if (ref == ref_tag)
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index 378b2459efe2..e253e7bd0d17 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -20,6 +20,7 @@ struct blk_integrity_iter {
 	unsigned int		data_size;
 	unsigned short		interval;
 	unsigned char		tuple_size;
+	unsigned char		pi_offset;
 	const char		*disk_name;
 };
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d7cac3de65b3..bb4d811fee46 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -108,6 +108,7 @@ struct blk_integrity {
 	const struct blk_integrity_profile	*profile;
 	unsigned char				flags;
 	unsigned char				tuple_size;
+	unsigned char				pi_offset;
 	unsigned char				interval_exp;
 	unsigned char				tag_size;
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] nvme: allow integrity when PI is not in first bytes
       [not found]   ` <CGME20240130171929epcas5p24f6c25d123d3cd6463cbef1aaf795276@epcas5p2.samsung.com>
@ 2024-01-30 17:12     ` Kanchan Joshi
  2024-01-31  7:22       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  0 siblings, 2 replies; 12+ messages in thread
From: Kanchan Joshi @ 2024-01-30 17:12 UTC (permalink / raw
  To: kbusch, axboe, hch, martin.petersen, sagi
  Cc: linux-nvme, linux-block, gost.dev, Kanchan Joshi

NVM command set 1.0 (or later) mandate PI to be in last bytes of
metadata. But this was not supported in the block-layer and driver
registered a nop profile.

Remove the restriction as the block integrity subsystem has grown the
ability to support it.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 drivers/nvme/host/core.c | 8 +++++++-
 drivers/nvme/host/nvme.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 85ab0fcf9e88..4945d6259a13 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1726,6 +1726,7 @@ static void nvme_init_integrity(struct gendisk *disk,
 	}
 
 	integrity.tuple_size = head->ms;
+	integrity.pi_offset = head->pi_offset;
 	blk_integrity_register(disk, &integrity);
 	blk_queue_max_integrity_segments(disk->queue, max_integrity_segments);
 }
@@ -1835,11 +1836,16 @@ static int nvme_init_ms(struct nvme_ctrl *ctrl, struct nvme_ns_head *head,
 free_data:
 	kfree(nvm);
 set_pi:
-	if (head->pi_size && (first || head->ms == head->pi_size))
+	if (head->pi_size && head->ms >= head->pi_size)
 		head->pi_type = id->dps & NVME_NS_DPS_PI_MASK;
 	else
 		head->pi_type = 0;
 
+	if (first)
+		head->pi_offset = 0;
+	else
+		head->pi_offset = head->ms - head->pi_size;
+
 	return ret;
 }
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 030c80818240..281657320c3a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -462,6 +462,7 @@ struct nvme_ns_head {
 	u16			ms;
 	u16			pi_size;
 	u8			pi_type;
+	u8			pi_offset;
 	u8			guard_type;
 	u16			sgs;
 	u32			sws;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] block: refactor guard helpers
  2024-01-30 17:12     ` [PATCH 1/3] block: refactor guard helpers Kanchan Joshi
@ 2024-01-31  7:16       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2024-01-31  7:16 UTC (permalink / raw
  To: Kanchan Joshi
  Cc: kbusch, axboe, hch, martin.petersen, sagi, linux-nvme,
	linux-block, gost.dev

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] block: support PI at non-zero offset within metadata
  2024-01-30 17:12     ` [PATCH 2/3] block: support PI at non-zero offset within metadata Kanchan Joshi
@ 2024-01-31  7:21       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2024-01-31  7:21 UTC (permalink / raw
  To: Kanchan Joshi
  Cc: kbusch, axboe, hch, martin.petersen, sagi, linux-nvme,
	linux-block, gost.dev, Chinmay Gameti

On Tue, Jan 30, 2024 at 10:42:05PM +0530, Kanchan Joshi wrote:
> The block integrity subsystem assumes that PI appears first in the
> metadata buffer.
> Abolish this assumption by adding the ability to handle PI starting at
> a non-zero offset.
> Calculation of the guard tag includes the metadata buffer up to this
> offset.

Maybe rewrite this as:

Block layer integrity processing currently assumes that protection
information is placed in the first bytes of each metadata block.
Remove this this limitation and include the metadata before the
protection information in the calculation of the guard tag.

>  {
>  	unsigned int i;
> +	u8 offset = iter->pi_offset;

Nit: I find it more readable if variables that are initialized at
declaration time come first.  Also (with a few exceptions) it's nice
to read longer declarations first.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] nvme: allow integrity when PI is not in first bytes
  2024-01-30 17:12     ` [PATCH 3/3] nvme: allow integrity when PI is not in first bytes Kanchan Joshi
@ 2024-01-31  7:22       ` Christoph Hellwig
  2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2024-01-31  7:22 UTC (permalink / raw
  To: Kanchan Joshi
  Cc: kbusch, axboe, hch, martin.petersen, sagi, linux-nvme,
	linux-block, gost.dev

On Tue, Jan 30, 2024 at 10:42:06PM +0530, Kanchan Joshi wrote:
> NVM command set 1.0 (or later) mandate PI to be in last bytes of
> metadata. But this was not supported in the block-layer and driver
> registered a nop profile.
> 
> Remove the restriction as the block integrity subsystem has grown the
> ability to support it.

I think it makes sense to mention that the by far most usual
configuration is metadata size == PI tuple size and you're adding
support for additional less common setups.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] block: refactor guard helpers
  2024-01-30 17:12     ` [PATCH 1/3] block: refactor guard helpers Kanchan Joshi
  2024-01-31  7:16       ` Christoph Hellwig
@ 2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Sagi Grimberg @ 2024-01-31 12:43 UTC (permalink / raw
  To: Kanchan Joshi, kbusch, axboe, hch, martin.petersen
  Cc: linux-nvme, linux-block, gost.dev

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] block: support PI at non-zero offset within metadata
  2024-01-30 17:12     ` [PATCH 2/3] block: support PI at non-zero offset within metadata Kanchan Joshi
  2024-01-31  7:21       ` Christoph Hellwig
@ 2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Sagi Grimberg @ 2024-01-31 12:43 UTC (permalink / raw
  To: Kanchan Joshi, kbusch, axboe, hch, martin.petersen
  Cc: linux-nvme, linux-block, gost.dev, Chinmay Gameti

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] nvme: allow integrity when PI is not in first bytes
  2024-01-30 17:12     ` [PATCH 3/3] nvme: allow integrity when PI is not in first bytes Kanchan Joshi
  2024-01-31  7:22       ` Christoph Hellwig
@ 2024-01-31 12:43       ` Sagi Grimberg
  1 sibling, 0 replies; 12+ messages in thread
From: Sagi Grimberg @ 2024-01-31 12:43 UTC (permalink / raw
  To: Kanchan Joshi, kbusch, axboe, hch, martin.petersen
  Cc: linux-nvme, linux-block, gost.dev

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Block integrity with flexibile-offset PI
  2024-01-30 17:12 ` [PATCH 0/3] Block integrity with flexibile-offset PI Kanchan Joshi
                     ` (2 preceding siblings ...)
       [not found]   ` <CGME20240130171929epcas5p24f6c25d123d3cd6463cbef1aaf795276@epcas5p2.samsung.com>
@ 2024-01-31 17:44   ` Keith Busch
  2024-01-31 17:48   ` Martin K. Petersen
  4 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2024-01-31 17:44 UTC (permalink / raw
  To: Kanchan Joshi
  Cc: axboe, hch, martin.petersen, sagi, linux-nvme, linux-block,
	gost.dev

On Tue, Jan 30, 2024 at 10:42:03PM +0530, Kanchan Joshi wrote:
> The block integrity subsystem can only work with PI placed in the first
> bytes of the metadata buffer.
> 
> The series makes block-integrity support the flexible placement of PI.
> And changes NVMe driver to make use of the new capability.
> 
> This helps to
> (i) enable the more common case for NVMe (PI in last bytes is the norm)
> (ii) reduce nop profile users (tried by Jens recently [1]).
> 
> /* For NS 4K+16b, 8b PI, last bytes */
> Before:
> # cat /sys/block/nvme0n1/integrity/format
> nop
> 
> After:
> # cat /sys/block/nvme0n1/integrity/format
> T10-DIF-TYPE1-CRC

Your series looks good to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>

While reviewing, I realized O_DIRECT with DMA aligned offsets smaller
than block sized offsets can create segments that break PI iterations.
Not your problem, just mentioning it because I noticed ... and since I
introduced DMA aligned offsets, I should probably take a shot at fixing
that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Block integrity with flexibile-offset PI
  2024-01-30 17:12 ` [PATCH 0/3] Block integrity with flexibile-offset PI Kanchan Joshi
                     ` (3 preceding siblings ...)
  2024-01-31 17:44   ` [PATCH 0/3] Block integrity with flexibile-offset PI Keith Busch
@ 2024-01-31 17:48   ` Martin K. Petersen
  4 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2024-01-31 17:48 UTC (permalink / raw
  To: Kanchan Joshi
  Cc: kbusch, axboe, hch, martin.petersen, sagi, linux-nvme,
	linux-block, gost.dev


Kanchan,

> The block integrity subsystem can only work with PI placed in the
> first bytes of the metadata buffer.
>
> The series makes block-integrity support the flexible placement of PI.
> And changes NVMe driver to make use of the new capability.
>
> This helps to
> (i) enable the more common case for NVMe (PI in last bytes is the norm)
> (ii) reduce nop profile users (tried by Jens recently [1]).

Looks OK to me.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-01-31 17:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20240130171918epcas5p3cd0e3e9c7fb9a74c8464b06779c378ea@epcas5p3.samsung.com>
2024-01-30 17:12 ` [PATCH 0/3] Block integrity with flexibile-offset PI Kanchan Joshi
     [not found]   ` <CGME20240130171923epcas5p4610296779861b362ef98f3b6f819a060@epcas5p4.samsung.com>
2024-01-30 17:12     ` [PATCH 1/3] block: refactor guard helpers Kanchan Joshi
2024-01-31  7:16       ` Christoph Hellwig
2024-01-31 12:43       ` Sagi Grimberg
     [not found]   ` <CGME20240130171927epcas5p2814181a20402d08b96393ce4a5e06e03@epcas5p2.samsung.com>
2024-01-30 17:12     ` [PATCH 2/3] block: support PI at non-zero offset within metadata Kanchan Joshi
2024-01-31  7:21       ` Christoph Hellwig
2024-01-31 12:43       ` Sagi Grimberg
     [not found]   ` <CGME20240130171929epcas5p24f6c25d123d3cd6463cbef1aaf795276@epcas5p2.samsung.com>
2024-01-30 17:12     ` [PATCH 3/3] nvme: allow integrity when PI is not in first bytes Kanchan Joshi
2024-01-31  7:22       ` Christoph Hellwig
2024-01-31 12:43       ` Sagi Grimberg
2024-01-31 17:44   ` [PATCH 0/3] Block integrity with flexibile-offset PI Keith Busch
2024-01-31 17:48   ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).