All of lore.kernel.org
 help / color / mirror / Atom feed
* Block/SCSI data integrity update v3
@ 2014-08-28 19:31 Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 01/14] block: Get rid of bdev_integrity_enabled() Martin K. Petersen
                   ` (14 more replies)
  0 siblings, 15 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig

This is the data integrity patch series originally submitted for 3.16
and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
than that there are no changes from v2.

-- 
Martin K. Petersen	Oracle Linux Engineering


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

* [PATCH 01/14] block: Get rid of bdev_integrity_enabled()
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 02/14] block: Replace bi_integrity with bi_special Martin K. Petersen
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

bdev_integrity_enabled() is only used by bio_integrity_enabled().
Combine these two functions.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 Documentation/block/data-integrity.txt | 10 ---------
 block/bio-integrity.c                  | 39 +++++++++++++++-------------------
 include/linux/bio.h                    |  6 +++---
 3 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/Documentation/block/data-integrity.txt b/Documentation/block/data-integrity.txt
index 2d735b0ae383..b4eacf48053c 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -192,16 +192,6 @@ will require extra work due to the application tag.
     supported by the block device.
 
 
-    int bdev_integrity_enabled(block_device, int rw);
-
-      bdev_integrity_enabled() will return 1 if the block device
-      supports integrity metadata transfer for the data direction
-      specified in 'rw'.
-
-      bdev_integrity_enabled() honors the write_generate and
-      read_verify flags in sysfs and will respond accordingly.
-
-
     int bio_integrity_prep(bio);
 
       To generate IMD for WRITE and to set up buffers for READ, the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index bc423f7b02da..d660d9ccc6b9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -147,24 +147,6 @@ int bio_integrity_add_page(struct bio *bio, struct page *page,
 }
 EXPORT_SYMBOL(bio_integrity_add_page);
 
-static int bdev_integrity_enabled(struct block_device *bdev, int rw)
-{
-	struct blk_integrity *bi = bdev_get_integrity(bdev);
-
-	if (bi == NULL)
-		return 0;
-
-	if (rw == READ && bi->verify_fn != NULL &&
-	    (bi->flags & INTEGRITY_FLAG_READ))
-		return 1;
-
-	if (rw == WRITE && bi->generate_fn != NULL &&
-	    (bi->flags & INTEGRITY_FLAG_WRITE))
-		return 1;
-
-	return 0;
-}
-
 /**
  * bio_integrity_enabled - Check whether integrity can be passed
  * @bio:	bio to check
@@ -174,16 +156,29 @@ static int bdev_integrity_enabled(struct block_device *bdev, int rw)
  * set prior to calling.  The functions honors the write_generate and
  * read_verify flags in sysfs.
  */
-int bio_integrity_enabled(struct bio *bio)
+bool bio_integrity_enabled(struct bio *bio)
 {
+	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
+
 	if (!bio_is_rw(bio))
-		return 0;
+		return false;
 
 	/* Already protected? */
 	if (bio_integrity(bio))
-		return 0;
+		return false;
+
+	if (bi == NULL)
+		return false;
+
+	if (bio_data_dir(bio) == READ && bi->verify_fn != NULL &&
+	    (bi->flags & INTEGRITY_FLAG_READ))
+		return true;
+
+	if (bio_data_dir(bio) == WRITE && bi->generate_fn != NULL &&
+	    (bi->flags & INTEGRITY_FLAG_WRITE))
+		return true;
 
-	return bdev_integrity_enabled(bio->bi_bdev, bio_data_dir(bio));
+	return false;
 }
 EXPORT_SYMBOL(bio_integrity_enabled);
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b39e5000ff58..63e399b4fde5 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -666,7 +666,7 @@ struct biovec_slab {
 extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t, unsigned int);
 extern void bio_integrity_free(struct bio *);
 extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int, unsigned int);
-extern int bio_integrity_enabled(struct bio *bio);
+extern bool bio_integrity_enabled(struct bio *bio);
 extern int bio_integrity_set_tag(struct bio *, void *, unsigned int);
 extern int bio_integrity_get_tag(struct bio *, void *, unsigned int);
 extern int bio_integrity_prep(struct bio *);
@@ -685,9 +685,9 @@ static inline int bio_integrity(struct bio *bio)
 	return 0;
 }
 
-static inline int bio_integrity_enabled(struct bio *bio)
+static inline bool bio_integrity_enabled(struct bio *bio)
 {
-	return 0;
+	return false;
 }
 
 static inline int bioset_integrity_create(struct bio_set *bs, int pool_size)
-- 
1.9.3


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

* [PATCH 02/14] block: Replace bi_integrity with bi_special
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 01/14] block: Get rid of bdev_integrity_enabled() Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 03/14] block: Remove integrity tagging functions Martin K. Petersen
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

For commands like REQ_COPY we need a way to pass extra information along
with each bio. Like integrity metadata this information must be
available at the bottom of the stack so bi_private does not suffice.

Rename the existing bi_integrity field to bi_special and make it a union
so we can have different bio extensions for each class of command.

We previously used bi_integrity != NULL as a way to identify whether a
bio had integrity metadata or not. Introduce a REQ_INTEGRITY to be the
indicator now that bi_special can contain different things.

In addition, bio_integrity(bio) will now return a pointer to the
integrity payload (when applicable).

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 Documentation/block/data-integrity.txt | 10 +++++-----
 block/bio-integrity.c                  | 19 ++++++++++---------
 drivers/scsi/sd_dif.c                  |  8 ++++----
 include/linux/bio.h                    | 11 +++++++++--
 include/linux/blk_types.h              |  8 ++++++--
 include/linux/blkdev.h                 |  7 ++-----
 6 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/Documentation/block/data-integrity.txt b/Documentation/block/data-integrity.txt
index b4eacf48053c..4d4de8b09530 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -129,11 +129,11 @@ interface for this is being worked on.
 4.1 BIO
 
 The data integrity patches add a new field to struct bio when
-CONFIG_BLK_DEV_INTEGRITY is enabled.  bio->bi_integrity is a pointer
-to a struct bip which contains the bio integrity payload.  Essentially
-a bip is a trimmed down struct bio which holds a bio_vec containing
-the integrity metadata and the required housekeeping information (bvec
-pool, vector count, etc.)
+CONFIG_BLK_DEV_INTEGRITY is enabled.  bio_integrity(bio) returns a
+pointer to a struct bip which contains the bio integrity payload.
+Essentially a bip is a trimmed down struct bio which holds a bio_vec
+containing the integrity metadata and the required housekeeping
+information (bvec pool, vector count, etc.)
 
 A kernel subsystem can enable data integrity protection on a bio by
 calling bio_integrity_alloc(bio).  This will allocate and attach the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index d660d9ccc6b9..148a63e1e323 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -79,6 +79,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio *bio,
 	bip->bip_slab = idx;
 	bip->bip_bio = bio;
 	bio->bi_integrity = bip;
+	bio->bi_rw |= REQ_INTEGRITY;
 
 	return bip;
 err:
@@ -96,7 +97,7 @@ EXPORT_SYMBOL(bio_integrity_alloc);
  */
 void bio_integrity_free(struct bio *bio)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct bio_set *bs = bio->bi_pool;
 
 	if (bip->bip_owns_buf)
@@ -128,7 +129,7 @@ EXPORT_SYMBOL(bio_integrity_free);
 int bio_integrity_add_page(struct bio *bio, struct page *page,
 			   unsigned int len, unsigned int offset)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct bio_vec *iv;
 
 	if (bip->bip_vcnt >= bip->bip_max_vcnt) {
@@ -229,7 +230,7 @@ EXPORT_SYMBOL(bio_integrity_tag_size);
 static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
 			     int set)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 	unsigned int nr_sectors;
 
@@ -304,12 +305,12 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 	struct bio_vec *bv;
 	sector_t sector;
 	unsigned int sectors, ret = 0, i;
-	void *prot_buf = bio->bi_integrity->bip_buf;
+	void *prot_buf = bio_integrity(bio)->bip_buf;
 
 	if (operate)
 		sector = bio->bi_iter.bi_sector;
 	else
-		sector = bio->bi_integrity->bip_iter.bi_sector;
+		sector = bio_integrity(bio)->bip_iter.bi_sector;
 
 	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
 	bix.sector_size = bi->sector_size;
@@ -505,7 +506,7 @@ static void bio_integrity_verify_fn(struct work_struct *work)
  */
 void bio_integrity_endio(struct bio *bio, int error)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 
 	BUG_ON(bip->bip_bio != bio);
 
@@ -536,7 +537,7 @@ EXPORT_SYMBOL(bio_integrity_endio);
  */
 void bio_integrity_advance(struct bio *bio, unsigned int bytes_done)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 	unsigned bytes = bio_integrity_bytes(bi, bytes_done >> 9);
 
@@ -558,7 +559,7 @@ EXPORT_SYMBOL(bio_integrity_advance);
 void bio_integrity_trim(struct bio *bio, unsigned int offset,
 			unsigned int sectors)
 {
-	struct bio_integrity_payload *bip = bio->bi_integrity;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 
 	bio_integrity_advance(bio, offset << 9);
@@ -577,7 +578,7 @@ EXPORT_SYMBOL(bio_integrity_trim);
 int bio_integrity_clone(struct bio *bio, struct bio *bio_src,
 			gfp_t gfp_mask)
 {
-	struct bio_integrity_payload *bip_src = bio_src->bi_integrity;
+	struct bio_integrity_payload *bip_src = bio_integrity(bio_src);
 	struct bio_integrity_payload *bip;
 
 	BUG_ON(bip_src == NULL);
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index a7a691d0af7d..29f0477a8708 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -383,9 +383,9 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 		if (bio_flagged(bio, BIO_MAPPED_INTEGRITY))
 			break;
 
-		virt = bio->bi_integrity->bip_iter.bi_sector & 0xffffffff;
+		virt = bio_integrity(bio)->bip_iter.bi_sector & 0xffffffff;
 
-		bip_for_each_vec(iv, bio->bi_integrity, iter) {
+		bip_for_each_vec(iv, bio_integrity(bio), iter) {
 			sdt = kmap_atomic(iv.bv_page)
 				+ iv.bv_offset;
 
@@ -434,9 +434,9 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 		struct bio_vec iv;
 		struct bvec_iter iter;
 
-		virt = bio->bi_integrity->bip_iter.bi_sector & 0xffffffff;
+		virt = bio_integrity(bio)->bip_iter.bi_sector & 0xffffffff;
 
-		bip_for_each_vec(iv, bio->bi_integrity, iter) {
+		bip_for_each_vec(iv, bio_integrity(bio), iter) {
 			sdt = kmap_atomic(iv.bv_page)
 				+ iv.bv_offset;
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 63e399b4fde5..a810a74071b2 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -293,6 +293,15 @@ static inline unsigned bio_segments(struct bio *bio)
 #define bio_get(bio)	atomic_inc(&(bio)->bi_cnt)
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
+
+static inline struct bio_integrity_payload *bio_integrity(struct bio *bio)
+{
+	if (bio->bi_rw & REQ_INTEGRITY)
+		return bio->bi_integrity;
+
+	return NULL;
+}
+
 /*
  * bio integrity payload
  */
@@ -661,8 +670,6 @@ struct biovec_slab {
 	for_each_bio(_bio)						\
 		bip_for_each_vec(_bvl, _bio->bi_integrity, _iter)
 
-#define bio_integrity(bio) (bio->bi_integrity != NULL)
-
 extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t, unsigned int);
 extern void bio_integrity_free(struct bio *);
 extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int, unsigned int);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 66c2167f04a9..0b7084f7c136 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -78,9 +78,11 @@ struct bio {
 	struct io_context	*bi_ioc;
 	struct cgroup_subsys_state *bi_css;
 #endif
+	union {
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
-	struct bio_integrity_payload *bi_integrity;  /* data integrity */
+		struct bio_integrity_payload *bi_integrity; /* data integrity */
 #endif
+	};
 
 	unsigned short		bi_vcnt;	/* how many bio_vec's */
 
@@ -162,6 +164,7 @@ enum rq_flag_bits {
 	__REQ_WRITE_SAME,	/* write same block many times */
 
 	__REQ_NOIDLE,		/* don't anticipate more IO after this one */
+	__REQ_INTEGRITY,	/* I/O includes block integrity payload */
 	__REQ_FUA,		/* forced unit access */
 	__REQ_FLUSH,		/* request for cache flush */
 
@@ -204,13 +207,14 @@ enum rq_flag_bits {
 #define REQ_DISCARD		(1ULL << __REQ_DISCARD)
 #define REQ_WRITE_SAME		(1ULL << __REQ_WRITE_SAME)
 #define REQ_NOIDLE		(1ULL << __REQ_NOIDLE)
+#define REQ_INTEGRITY		(1ULL << __REQ_INTEGRITY)
 
 #define REQ_FAILFAST_MASK \
 	(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
 #define REQ_COMMON_MASK \
 	(REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_PRIO | \
 	 REQ_DISCARD | REQ_WRITE_SAME | REQ_NOIDLE | REQ_FLUSH | REQ_FUA | \
-	 REQ_SECURE)
+	 REQ_SECURE | REQ_INTEGRITY)
 #define REQ_CLONE_MASK		REQ_COMMON_MASK
 
 #define BIO_NO_ADVANCE_ITER_MASK	(REQ_DISCARD|REQ_WRITE_SAME)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 518b46555b80..f8e52ec150e5 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1520,12 +1520,9 @@ static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
 	return disk->integrity;
 }
 
-static inline int blk_integrity_rq(struct request *rq)
+static inline bool blk_integrity_rq(struct request *rq)
 {
-	if (rq->bio == NULL)
-		return 0;
-
-	return bio_integrity(rq->bio);
+	return rq->cmd_flags & REQ_INTEGRITY;
 }
 
 static inline void blk_queue_max_integrity_segments(struct request_queue *q,
-- 
1.9.3


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

* [PATCH 03/14] block: Remove integrity tagging functions
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 01/14] block: Get rid of bdev_integrity_enabled() Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 02/14] block: Replace bi_integrity with bi_special Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 04/14] block: Remove bip_buf Martin K. Petersen
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

None of the filesystems appear interested in using the integrity tagging
feature. Potentially because very few storage devices actually permit
using the application tag space.

Remove the tagging functions.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 Documentation/block/data-integrity.txt | 34 ------------
 block/bio-integrity.c                  | 94 +---------------------------------
 block/blk-integrity.c                  |  2 -
 drivers/scsi/sd_dif.c                  | 65 -----------------------
 include/linux/bio.h                    |  3 --
 include/linux/blkdev.h                 |  4 --
 6 files changed, 1 insertion(+), 201 deletions(-)

diff --git a/Documentation/block/data-integrity.txt b/Documentation/block/data-integrity.txt
index 4d4de8b09530..f56ec97f0d14 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -206,36 +206,6 @@ will require extra work due to the application tag.
       bio_integrity_enabled() returned 1.
 
 
-    int bio_integrity_tag_size(bio);
-
-      If the filesystem wants to use the application tag space it will
-      first have to find out how much storage space is available.
-      Because tag space is generally limited (usually 2 bytes per
-      sector regardless of sector size), the integrity framework
-      supports interleaving the information between the sectors in an
-      I/O.
-
-      Filesystems can call bio_integrity_tag_size(bio) to find out how
-      many bytes of storage are available for that particular bio.
-
-      Another option is bdev_get_tag_size(block_device) which will
-      return the number of available bytes per hardware sector.
-
-
-    int bio_integrity_set_tag(bio, void *tag_buf, len);
-
-      After a successful return from bio_integrity_prep(),
-      bio_integrity_set_tag() can be used to attach an opaque tag
-      buffer to a bio.  Obviously this only makes sense if the I/O is
-      a WRITE.
-
-
-    int bio_integrity_get_tag(bio, void *tag_buf, len);
-
-      Similarly, at READ I/O completion time the filesystem can
-      retrieve the tag buffer using bio_integrity_get_tag().
-
-
 5.3 PASSING EXISTING INTEGRITY METADATA
 
     Filesystems that either generate their own integrity metadata or
@@ -288,8 +258,6 @@ will require extra work due to the application tag.
             .name                   = "STANDARDSBODY-TYPE-VARIANT-CSUM",
             .generate_fn            = my_generate_fn,
        	    .verify_fn              = my_verify_fn,
-       	    .get_tag_fn             = my_get_tag_fn,
-       	    .set_tag_fn             = my_set_tag_fn,
 	    .tuple_size             = sizeof(struct my_tuple_size),
 	    .tag_size               = <tag bytes per hw sector>,
         };
@@ -311,7 +279,5 @@ will require extra work due to the application tag.
       are available per hardware sector.  For DIF this is either 2 or
       0 depending on the value of the Control Mode Page ATO bit.
 
-      See 6.2 for a description of get_tag_fn and set_tag_fn.
-
 ----------------------------------------------------------------------
 2007-12-24 Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 148a63e1e323..9be2431475e5 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -210,90 +210,6 @@ static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
 }
 
 /**
- * bio_integrity_tag_size - Retrieve integrity tag space
- * @bio:	bio to inspect
- *
- * Description: Returns the maximum number of tag bytes that can be
- * attached to this bio. Filesystems can use this to determine how
- * much metadata to attach to an I/O.
- */
-unsigned int bio_integrity_tag_size(struct bio *bio)
-{
-	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
-
-	BUG_ON(bio->bi_iter.bi_size == 0);
-
-	return bi->tag_size * (bio->bi_iter.bi_size / bi->sector_size);
-}
-EXPORT_SYMBOL(bio_integrity_tag_size);
-
-static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
-			     int set)
-{
-	struct bio_integrity_payload *bip = bio_integrity(bio);
-	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
-	unsigned int nr_sectors;
-
-	BUG_ON(bip->bip_buf == NULL);
-
-	if (bi->tag_size == 0)
-		return -1;
-
-	nr_sectors = bio_integrity_hw_sectors(bi,
-					DIV_ROUND_UP(len, bi->tag_size));
-
-	if (nr_sectors * bi->tuple_size > bip->bip_iter.bi_size) {
-		printk(KERN_ERR "%s: tag too big for bio: %u > %u\n", __func__,
-		       nr_sectors * bi->tuple_size, bip->bip_iter.bi_size);
-		return -1;
-	}
-
-	if (set)
-		bi->set_tag_fn(bip->bip_buf, tag_buf, nr_sectors);
-	else
-		bi->get_tag_fn(bip->bip_buf, tag_buf, nr_sectors);
-
-	return 0;
-}
-
-/**
- * bio_integrity_set_tag - Attach a tag buffer to a bio
- * @bio:	bio to attach buffer to
- * @tag_buf:	Pointer to a buffer containing tag data
- * @len:	Length of the included buffer
- *
- * Description: Use this function to tag a bio by leveraging the extra
- * space provided by devices formatted with integrity protection.  The
- * size of the integrity buffer must be <= to the size reported by
- * bio_integrity_tag_size().
- */
-int bio_integrity_set_tag(struct bio *bio, void *tag_buf, unsigned int len)
-{
-	BUG_ON(bio_data_dir(bio) != WRITE);
-
-	return bio_integrity_tag(bio, tag_buf, len, 1);
-}
-EXPORT_SYMBOL(bio_integrity_set_tag);
-
-/**
- * bio_integrity_get_tag - Retrieve a tag buffer from a bio
- * @bio:	bio to retrieve buffer from
- * @tag_buf:	Pointer to a buffer for the tag data
- * @len:	Length of the target buffer
- *
- * Description: Use this function to retrieve the tag buffer from a
- * completed I/O. The size of the integrity buffer must be <= to the
- * size reported by bio_integrity_tag_size().
- */
-int bio_integrity_get_tag(struct bio *bio, void *tag_buf, unsigned int len)
-{
-	BUG_ON(bio_data_dir(bio) != READ);
-
-	return bio_integrity_tag(bio, tag_buf, len, 0);
-}
-EXPORT_SYMBOL(bio_integrity_get_tag);
-
-/**
  * bio_integrity_generate_verify - Generate/verify integrity metadata for a bio
  * @bio:	bio to generate/verify integrity metadata for
  * @operate:	operate number, 1 for generate, 0 for verify
@@ -355,14 +271,6 @@ static void bio_integrity_generate(struct bio *bio)
 	bio_integrity_generate_verify(bio, 1);
 }
 
-static inline unsigned short blk_integrity_tuple_size(struct blk_integrity *bi)
-{
-	if (bi)
-		return bi->tuple_size;
-
-	return 0;
-}
-
 /**
  * bio_integrity_prep - Prepare bio for integrity I/O
  * @bio:	bio to prepare
@@ -393,7 +301,7 @@ int bio_integrity_prep(struct bio *bio)
 	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
 
 	/* Allocate kernel buffer for protection data */
-	len = sectors * blk_integrity_tuple_size(bi);
+	len = sectors * bi->tuple_size;
 	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7fbab84399e6..7ac17160ab69 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -418,8 +418,6 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 		bi->generate_fn = template->generate_fn;
 		bi->verify_fn = template->verify_fn;
 		bi->tuple_size = template->tuple_size;
-		bi->set_tag_fn = template->set_tag_fn;
-		bi->get_tag_fn = template->get_tag_fn;
 		bi->tag_size = template->tag_size;
 	} else
 		bi->name = bi_unsupported_name;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 29f0477a8708..38a7778631be 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -128,39 +128,10 @@ static int sd_dif_type1_verify_ip(struct blk_integrity_exchg *bix)
 	return sd_dif_type1_verify(bix, sd_dif_ip_fn);
 }
 
-/*
- * Functions for interleaving and deinterleaving application tags
- */
-static void sd_dif_type1_set_tag(void *prot, void *tag_buf, unsigned int sectors)
-{
-	struct sd_dif_tuple *sdt = prot;
-	u8 *tag = tag_buf;
-	unsigned int i, j;
-
-	for (i = 0, j = 0 ; i < sectors ; i++, j += 2, sdt++) {
-		sdt->app_tag = tag[j] << 8 | tag[j+1];
-		BUG_ON(sdt->app_tag == 0xffff);
-	}
-}
-
-static void sd_dif_type1_get_tag(void *prot, void *tag_buf, unsigned int sectors)
-{
-	struct sd_dif_tuple *sdt = prot;
-	u8 *tag = tag_buf;
-	unsigned int i, j;
-
-	for (i = 0, j = 0 ; i < sectors ; i++, j += 2, sdt++) {
-		tag[j] = (sdt->app_tag & 0xff00) >> 8;
-		tag[j+1] = sdt->app_tag & 0xff;
-	}
-}
-
 static struct blk_integrity dif_type1_integrity_crc = {
 	.name			= "T10-DIF-TYPE1-CRC",
 	.generate_fn		= sd_dif_type1_generate_crc,
 	.verify_fn		= sd_dif_type1_verify_crc,
-	.get_tag_fn		= sd_dif_type1_get_tag,
-	.set_tag_fn		= sd_dif_type1_set_tag,
 	.tuple_size		= sizeof(struct sd_dif_tuple),
 	.tag_size		= 0,
 };
@@ -169,8 +140,6 @@ static struct blk_integrity dif_type1_integrity_ip = {
 	.name			= "T10-DIF-TYPE1-IP",
 	.generate_fn		= sd_dif_type1_generate_ip,
 	.verify_fn		= sd_dif_type1_verify_ip,
-	.get_tag_fn		= sd_dif_type1_get_tag,
-	.set_tag_fn		= sd_dif_type1_set_tag,
 	.tuple_size		= sizeof(struct sd_dif_tuple),
 	.tag_size		= 0,
 };
@@ -245,42 +214,10 @@ static int sd_dif_type3_verify_ip(struct blk_integrity_exchg *bix)
 	return sd_dif_type3_verify(bix, sd_dif_ip_fn);
 }
 
-static void sd_dif_type3_set_tag(void *prot, void *tag_buf, unsigned int sectors)
-{
-	struct sd_dif_tuple *sdt = prot;
-	u8 *tag = tag_buf;
-	unsigned int i, j;
-
-	for (i = 0, j = 0 ; i < sectors ; i++, j += 6, sdt++) {
-		sdt->app_tag = tag[j] << 8 | tag[j+1];
-		sdt->ref_tag = tag[j+2] << 24 | tag[j+3] << 16 |
-			tag[j+4] << 8 | tag[j+5];
-	}
-}
-
-static void sd_dif_type3_get_tag(void *prot, void *tag_buf, unsigned int sectors)
-{
-	struct sd_dif_tuple *sdt = prot;
-	u8 *tag = tag_buf;
-	unsigned int i, j;
-
-	for (i = 0, j = 0 ; i < sectors ; i++, j += 2, sdt++) {
-		tag[j] = (sdt->app_tag & 0xff00) >> 8;
-		tag[j+1] = sdt->app_tag & 0xff;
-		tag[j+2] = (sdt->ref_tag & 0xff000000) >> 24;
-		tag[j+3] = (sdt->ref_tag & 0xff0000) >> 16;
-		tag[j+4] = (sdt->ref_tag & 0xff00) >> 8;
-		tag[j+5] = sdt->ref_tag & 0xff;
-		BUG_ON(sdt->app_tag == 0xffff || sdt->ref_tag == 0xffffffff);
-	}
-}
-
 static struct blk_integrity dif_type3_integrity_crc = {
 	.name			= "T10-DIF-TYPE3-CRC",
 	.generate_fn		= sd_dif_type3_generate_crc,
 	.verify_fn		= sd_dif_type3_verify_crc,
-	.get_tag_fn		= sd_dif_type3_get_tag,
-	.set_tag_fn		= sd_dif_type3_set_tag,
 	.tuple_size		= sizeof(struct sd_dif_tuple),
 	.tag_size		= 0,
 };
@@ -289,8 +226,6 @@ static struct blk_integrity dif_type3_integrity_ip = {
 	.name			= "T10-DIF-TYPE3-IP",
 	.generate_fn		= sd_dif_type3_generate_ip,
 	.verify_fn		= sd_dif_type3_verify_ip,
-	.get_tag_fn		= sd_dif_type3_get_tag,
-	.set_tag_fn		= sd_dif_type3_set_tag,
 	.tuple_size		= sizeof(struct sd_dif_tuple),
 	.tag_size		= 0,
 };
diff --git a/include/linux/bio.h b/include/linux/bio.h
index a810a74071b2..63a0e53e238c 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -362,7 +362,6 @@ extern struct bio *bio_clone_fast(struct bio *, gfp_t, struct bio_set *);
 extern struct bio *bio_clone_bioset(struct bio *, gfp_t, struct bio_set *bs);
 
 extern struct bio_set *fs_bio_set;
-unsigned int bio_integrity_tag_size(struct bio *bio);
 
 static inline struct bio *bio_alloc(gfp_t gfp_mask, unsigned int nr_iovecs)
 {
@@ -674,8 +673,6 @@ extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t, un
 extern void bio_integrity_free(struct bio *);
 extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int, unsigned int);
 extern bool bio_integrity_enabled(struct bio *bio);
-extern int bio_integrity_set_tag(struct bio *, void *, unsigned int);
-extern int bio_integrity_get_tag(struct bio *, void *, unsigned int);
 extern int bio_integrity_prep(struct bio *);
 extern void bio_integrity_endio(struct bio *, int);
 extern void bio_integrity_advance(struct bio *, unsigned int);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f8e52ec150e5..9e814ab51dfc 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1478,14 +1478,10 @@ struct blk_integrity_exchg {
 
 typedef void (integrity_gen_fn) (struct blk_integrity_exchg *);
 typedef int (integrity_vrfy_fn) (struct blk_integrity_exchg *);
-typedef void (integrity_set_tag_fn) (void *, void *, unsigned int);
-typedef void (integrity_get_tag_fn) (void *, void *, unsigned int);
 
 struct blk_integrity {
 	integrity_gen_fn	*generate_fn;
 	integrity_vrfy_fn	*verify_fn;
-	integrity_set_tag_fn	*set_tag_fn;
-	integrity_get_tag_fn	*get_tag_fn;
 
 	unsigned short		flags;
 	unsigned short		tuple_size;
-- 
1.9.3


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

* [PATCH 04/14] block: Remove bip_buf
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (2 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 03/14] block: Remove integrity tagging functions Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

bip_buf is not really needed so we can remove it.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c | 10 ++++++----
 include/linux/bio.h   |  3 ---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9be2431475e5..e4a015f661fc 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -101,7 +101,8 @@ void bio_integrity_free(struct bio *bio)
 	struct bio_set *bs = bio->bi_pool;
 
 	if (bip->bip_owns_buf)
-		kfree(bip->bip_buf);
+		kfree(page_address(bip->bip_vec->bv_page) +
+		      bip->bip_vec->bv_offset);
 
 	if (bs) {
 		if (bip->bip_slab != BIO_POOL_NONE)
@@ -219,14 +220,16 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 	struct blk_integrity_exchg bix;
 	struct bio_vec *bv;
+	struct bio_integrity_payload *bip = bio_integrity(bio);
 	sector_t sector;
 	unsigned int sectors, ret = 0, i;
-	void *prot_buf = bio_integrity(bio)->bip_buf;
+	void *prot_buf = page_address(bip->bip_vec->bv_page) +
+		bip->bip_vec->bv_offset;
 
 	if (operate)
 		sector = bio->bi_iter.bi_sector;
 	else
-		sector = bio_integrity(bio)->bip_iter.bi_sector;
+		sector = bip->bip_iter.bi_sector;
 
 	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
 	bix.sector_size = bi->sector_size;
@@ -321,7 +324,6 @@ int bio_integrity_prep(struct bio *bio)
 	}
 
 	bip->bip_owns_buf = 1;
-	bip->bip_buf = buf;
 	bip->bip_iter.bi_size = len;
 	bip->bip_iter.bi_sector = bio->bi_iter.bi_sector;
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 63a0e53e238c..448d8c052cb7 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -310,9 +310,6 @@ struct bio_integrity_payload {
 
 	struct bvec_iter	bip_iter;
 
-	/* kill - should just use bip_vec */
-	void			*bip_buf;	/* generated integrity data */
-
 	bio_end_io_t		*bip_end_io;	/* saved I/O completion fn */
 
 	unsigned short		bip_slab;	/* slab the bip came from */
-- 
1.9.3


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

* [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (3 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 04/14] block: Remove bip_buf Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 06/14] block: Make protection interval calculation generic Martin K. Petersen
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms "sector" and "sectors".

Going forward we will use the term "seed" to describe the initial
reference tag value for a given I/O. "Interval" will be used to describe
the portion of the data buffer that a given piece of protection
information is associated with.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c  | 42 +++++++++++++++++++++---------------------
 block/blk-integrity.c  | 10 +++++-----
 drivers/scsi/sd_dif.c  | 46 +++++++++++++++++++++++-----------------------
 include/linux/blkdev.h |  6 +++---
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e4a015f661fc..a2b3e994b55b 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -185,20 +185,20 @@ bool bio_integrity_enabled(struct bio *bio)
 EXPORT_SYMBOL(bio_integrity_enabled);
 
 /**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
  * @bi:		blk_integrity profile for device
- * @sectors:	Number of 512 sectors to convert
+ * @sectors:	Size of the bio in 512-byte sectors
  *
  * Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device.  Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device.  Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
  */
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
-						    unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+						   unsigned int sectors)
 {
 	/* At this point there are only 512b or 4096b DIF/EPP devices */
-	if (bi->sector_size == 4096)
+	if (bi->interval == 4096)
 		return sectors >>= 3;
 
 	return sectors;
@@ -207,7 +207,7 @@ static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
 static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
 					       unsigned int sectors)
 {
-	return bio_integrity_hw_sectors(bi, sectors) * bi->tuple_size;
+	return bio_integrity_intervals(bi, sectors) * bi->tuple_size;
 }
 
 /**
@@ -221,25 +221,25 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 	struct blk_integrity_exchg bix;
 	struct bio_vec *bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	sector_t sector;
-	unsigned int sectors, ret = 0, i;
+	sector_t seed;
+	unsigned int intervals, ret = 0, i;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
 
 	if (operate)
-		sector = bio->bi_iter.bi_sector;
+		seed = bio->bi_iter.bi_sector;
 	else
-		sector = bip->bip_iter.bi_sector;
+		seed = bip->bip_iter.bi_sector;
 
 	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
-	bix.sector_size = bi->sector_size;
+	bix.interval = bi->interval;
 
 	bio_for_each_segment_all(bv, bio, i) {
 		void *kaddr = kmap_atomic(bv->bv_page);
 		bix.data_buf = kaddr + bv->bv_offset;
 		bix.data_size = bv->bv_len;
 		bix.prot_buf = prot_buf;
-		bix.sector = sector;
+		bix.seed = seed;
 
 		if (operate)
 			bi->generate_fn(&bix);
@@ -251,9 +251,9 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 			}
 		}
 
-		sectors = bv->bv_len / bi->sector_size;
-		sector += sectors;
-		prot_buf += sectors * bi->tuple_size;
+		intervals = bv->bv_len / bi->interval;
+		seed += intervals;
+		prot_buf += intervals * bi->tuple_size;
 
 		kunmap_atomic(kaddr);
 	}
@@ -294,17 +294,17 @@ int bio_integrity_prep(struct bio *bio)
 	unsigned long start, end;
 	unsigned int len, nr_pages;
 	unsigned int bytes, offset, i;
-	unsigned int sectors;
+	unsigned int intervals;
 
 	bi = bdev_get_integrity(bio->bi_bdev);
 	q = bdev_get_queue(bio->bi_bdev);
 	BUG_ON(bi == NULL);
 	BUG_ON(bio_integrity(bio));
 
-	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
+	intervals = bio_integrity_intervals(bi, bio_sectors(bio));
 
 	/* Allocate kernel buffer for protection data */
-	len = sectors * bi->tuple_size;
+	len = intervals * bi->tuple_size;
 	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7ac17160ab69..3a83a7d08177 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -154,10 +154,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
 	if (!b1 || !b2)
 		return -1;
 
-	if (b1->sector_size != b2->sector_size) {
-		printk(KERN_ERR "%s: %s/%s sector sz %u != %u\n", __func__,
-		       gd1->disk_name, gd2->disk_name,
-		       b1->sector_size, b2->sector_size);
+	if (b1->interval != b2->interval) {
+		pr_err("%s: %s/%s protection interval %u != %u\n",
+		       __func__, gd1->disk_name, gd2->disk_name,
+		       b1->interval, b2->interval);
 		return -1;
 	}
 
@@ -407,7 +407,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 		kobject_uevent(&bi->kobj, KOBJ_ADD);
 
 		bi->flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
-		bi->sector_size = queue_logical_block_size(disk->queue);
+		bi->interval = queue_logical_block_size(disk->queue);
 		disk->integrity = bi;
 	} else
 		bi = disk->integrity;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 38a7778631be..1600270a46e5 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -57,16 +57,16 @@ static void sd_dif_type1_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
-		sdt->ref_tag = cpu_to_be32(sector & 0xffffffff);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
+		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 }
 
@@ -84,35 +84,35 @@ static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff)
 			return 0;
 
-		if (be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
+		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
 			printk(KERN_ERR
 			       "%s: ref tag error on sector %lu (rcvd %u)\n",
-			       bix->disk_name, (unsigned long)sector,
+			       bix->disk_name, (unsigned long)seed,
 			       be32_to_cpu(sdt->ref_tag));
 			return -EIO;
 		}
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
@@ -155,12 +155,12 @@ static void sd_dif_type3_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 	struct sd_dif_tuple *sdt = bix->prot_buf;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
 		sdt->ref_tag = 0;
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
+		buf += bix->interval;
 	}
 }
 
@@ -178,27 +178,27 @@ static int sd_dif_type3_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
 			return 0;
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9e814ab51dfc..41ea989b1d0d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1470,9 +1470,9 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 struct blk_integrity_exchg {
 	void			*prot_buf;
 	void			*data_buf;
-	sector_t		sector;
+	sector_t		seed;
 	unsigned int		data_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	const char		*disk_name;
 };
 
@@ -1485,7 +1485,7 @@ struct blk_integrity {
 
 	unsigned short		flags;
 	unsigned short		tuple_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	unsigned short		tag_size;
 
 	const char		*name;
-- 
1.9.3


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

* [PATCH 06/14] block: Make protection interval calculation generic
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (4 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 07/14] block: Clean up the code used to generate and verify integrity metadata Martin K. Petersen
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

Now that the protection interval has been detached from the sector size
we need to be able to handle sizes that are different from 4K and
512. Make the interval calculation generic.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index a2b3e994b55b..396244b1e774 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -197,11 +197,7 @@ EXPORT_SYMBOL(bio_integrity_enabled);
 static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
 						   unsigned int sectors)
 {
-	/* At this point there are only 512b or 4096b DIF/EPP devices */
-	if (bi->interval == 4096)
-		return sectors >>= 3;
-
-	return sectors;
+	return sectors >> (ilog2(bi->interval) - 9);
 }
 
 static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
-- 
1.9.3


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

* [PATCH 07/14] block: Clean up the code used to generate and verify integrity metadata
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (5 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 06/14] block: Make protection interval calculation generic Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 08/14] block: Add prefix to block integrity profile flags Martin K. Petersen
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

Instead of the "operate" parameter we pass in a seed value and a pointer
to a function that can be used to process the integrity metadata. The
generation function is changed to have a return value to fit into this
scheme.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c  |  82 ++++++++++----------------------------
 drivers/scsi/sd_dif.c  | 106 ++++++++++++++++++++++++++-----------------------
 include/linux/bio.h    |  12 ++++++
 include/linux/blkdev.h |   9 ++---
 4 files changed, 94 insertions(+), 115 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 396244b1e774..5ef996e8063f 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -207,49 +207,37 @@ static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
 }
 
 /**
- * bio_integrity_generate_verify - Generate/verify integrity metadata for a bio
+ * bio_integrity_process - Process integrity metadata for a bio
  * @bio:	bio to generate/verify integrity metadata for
- * @operate:	operate number, 1 for generate, 0 for verify
+ * @proc_fn:	Pointer to the relevant processing function
  */
-static int bio_integrity_generate_verify(struct bio *bio, int operate)
+static int bio_integrity_process(struct bio *bio,
+				 integrity_processing_fn *proc_fn)
 {
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
-	struct blk_integrity_exchg bix;
+	struct blk_integrity_iter iter;
 	struct bio_vec *bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	sector_t seed;
-	unsigned int intervals, ret = 0, i;
+	unsigned int i, ret = 0;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
 
-	if (operate)
-		seed = bio->bi_iter.bi_sector;
-	else
-		seed = bip->bip_iter.bi_sector;
-
-	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
-	bix.interval = bi->interval;
+	iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
+	iter.interval = bi->interval;
+	iter.seed = bip_get_seed(bip);
+	iter.prot_buf = prot_buf;
 
 	bio_for_each_segment_all(bv, bio, i) {
 		void *kaddr = kmap_atomic(bv->bv_page);
-		bix.data_buf = kaddr + bv->bv_offset;
-		bix.data_size = bv->bv_len;
-		bix.prot_buf = prot_buf;
-		bix.seed = seed;
-
-		if (operate)
-			bi->generate_fn(&bix);
-		else {
-			ret = bi->verify_fn(&bix);
-			if (ret) {
-				kunmap_atomic(kaddr);
-				return ret;
-			}
-		}
 
-		intervals = bv->bv_len / bi->interval;
-		seed += intervals;
-		prot_buf += intervals * bi->tuple_size;
+		iter.data_buf = kaddr + bv->bv_offset;
+		iter.data_size = bv->bv_len;
+
+		ret = proc_fn(&iter);
+		if (ret) {
+			kunmap_atomic(kaddr);
+			return ret;
+		}
 
 		kunmap_atomic(kaddr);
 	}
@@ -257,20 +245,6 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 }
 
 /**
- * bio_integrity_generate - Generate integrity metadata for a bio
- * @bio:	bio to generate integrity metadata for
- *
- * Description: Generates integrity metadata for a bio by calling the
- * block device's generation callback function.  The bio must have a
- * bip attached with enough room to accommodate the generated
- * integrity metadata.
- */
-static void bio_integrity_generate(struct bio *bio)
-{
-	bio_integrity_generate_verify(bio, 1);
-}
-
-/**
  * bio_integrity_prep - Prepare bio for integrity I/O
  * @bio:	bio to prepare
  *
@@ -321,7 +295,7 @@ int bio_integrity_prep(struct bio *bio)
 
 	bip->bip_owns_buf = 1;
 	bip->bip_iter.bi_size = len;
-	bip->bip_iter.bi_sector = bio->bi_iter.bi_sector;
+	bip_set_seed(bip, bio->bi_iter.bi_sector);
 
 	/* Map it */
 	offset = offset_in_page(buf);
@@ -357,26 +331,13 @@ int bio_integrity_prep(struct bio *bio)
 
 	/* Auto-generate integrity metadata if this is a write */
 	if (bio_data_dir(bio) == WRITE)
-		bio_integrity_generate(bio);
+		bio_integrity_process(bio, bi->generate_fn);
 
 	return 0;
 }
 EXPORT_SYMBOL(bio_integrity_prep);
 
 /**
- * bio_integrity_verify - Verify integrity metadata for a bio
- * @bio:	bio to verify
- *
- * Description: This function is called to verify the integrity of a
- * bio.	 The data in the bio io_vec is compared to the integrity
- * metadata returned by the HBA.
- */
-static int bio_integrity_verify(struct bio *bio)
-{
-	return bio_integrity_generate_verify(bio, 0);
-}
-
-/**
  * bio_integrity_verify_fn - Integrity I/O completion worker
  * @work:	Work struct stored in bio to be verified
  *
@@ -389,9 +350,10 @@ static void bio_integrity_verify_fn(struct work_struct *work)
 	struct bio_integrity_payload *bip =
 		container_of(work, struct bio_integrity_payload, bip_work);
 	struct bio *bio = bip->bip_bio;
+	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 	int error;
 
-	error = bio_integrity_verify(bio);
+	error = bio_integrity_process(bio, bi->verify_fn);
 
 	/* Restore original bio completion handler */
 	bio->bi_end_io = bip->bip_end_io;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 1600270a46e5..801c41851a01 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -53,42 +53,44 @@ static __u16 sd_dif_ip_fn(void *data, unsigned int len)
  * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
  * 16 bit app tag, 32 bit reference tag.
  */
-static void sd_dif_type1_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
+static void sd_dif_type1_generate(struct blk_integrity_iter *iter, csum_fn *fn)
 {
-	void *buf = bix->data_buf;
-	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t seed = bix->seed;
+	void *buf = iter->data_buf;
+	struct sd_dif_tuple *sdt = iter->prot_buf;
+	sector_t seed = iter->seed;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
-		sdt->guard_tag = fn(buf, bix->interval);
+	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
+		sdt->guard_tag = fn(buf, iter->interval);
 		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
 		sdt->app_tag = 0;
 
-		buf += bix->interval;
+		buf += iter->interval;
 		seed++;
 	}
 }
 
-static void sd_dif_type1_generate_crc(struct blk_integrity_exchg *bix)
+static int sd_dif_type1_generate_crc(struct blk_integrity_iter *iter)
 {
-	sd_dif_type1_generate(bix, sd_dif_crc_fn);
+	sd_dif_type1_generate(iter, sd_dif_crc_fn);
+	return 0;
 }
 
-static void sd_dif_type1_generate_ip(struct blk_integrity_exchg *bix)
+static int sd_dif_type1_generate_ip(struct blk_integrity_iter *iter)
 {
-	sd_dif_type1_generate(bix, sd_dif_ip_fn);
+	sd_dif_type1_generate(iter, sd_dif_ip_fn);
+	return 0;
 }
 
-static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
+static int sd_dif_type1_verify(struct blk_integrity_iter *iter, csum_fn *fn)
 {
-	void *buf = bix->data_buf;
-	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t seed = bix->seed;
+	void *buf = iter->data_buf;
+	struct sd_dif_tuple *sdt = iter->prot_buf;
+	sector_t seed = iter->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff)
 			return 0;
@@ -96,36 +98,36 @@ static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
 			printk(KERN_ERR
 			       "%s: ref tag error on sector %lu (rcvd %u)\n",
-			       bix->disk_name, (unsigned long)seed,
+			       iter->disk_name, (unsigned long)seed,
 			       be32_to_cpu(sdt->ref_tag));
 			return -EIO;
 		}
 
-		csum = fn(buf, bix->interval);
+		csum = fn(buf, iter->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
-			       "(rcvd %04x, data %04x)\n", bix->disk_name,
+			       "(rcvd %04x, data %04x)\n", iter->disk_name,
 			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->interval;
+		buf += iter->interval;
 		seed++;
 	}
 
 	return 0;
 }
 
-static int sd_dif_type1_verify_crc(struct blk_integrity_exchg *bix)
+static int sd_dif_type1_verify_crc(struct blk_integrity_iter *iter)
 {
-	return sd_dif_type1_verify(bix, sd_dif_crc_fn);
+	return sd_dif_type1_verify(iter, sd_dif_crc_fn);
 }
 
-static int sd_dif_type1_verify_ip(struct blk_integrity_exchg *bix)
+static int sd_dif_type1_verify_ip(struct blk_integrity_iter *iter)
 {
-	return sd_dif_type1_verify(bix, sd_dif_ip_fn);
+	return sd_dif_type1_verify(iter, sd_dif_ip_fn);
 }
 
 static struct blk_integrity dif_type1_integrity_crc = {
@@ -149,69 +151,71 @@ static struct blk_integrity dif_type1_integrity_ip = {
  * Type 3 protection has a 16-bit guard tag and 16 + 32 bits of opaque
  * tag space.
  */
-static void sd_dif_type3_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
+static void sd_dif_type3_generate(struct blk_integrity_iter *iter, csum_fn *fn)
 {
-	void *buf = bix->data_buf;
-	struct sd_dif_tuple *sdt = bix->prot_buf;
+	void *buf = iter->data_buf;
+	struct sd_dif_tuple *sdt = iter->prot_buf;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
-		sdt->guard_tag = fn(buf, bix->interval);
+	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
+		sdt->guard_tag = fn(buf, iter->interval);
 		sdt->ref_tag = 0;
 		sdt->app_tag = 0;
 
-		buf += bix->interval;
+		buf += iter->interval;
 	}
 }
 
-static void sd_dif_type3_generate_crc(struct blk_integrity_exchg *bix)
+static int sd_dif_type3_generate_crc(struct blk_integrity_iter *iter)
 {
-	sd_dif_type3_generate(bix, sd_dif_crc_fn);
+	sd_dif_type3_generate(iter, sd_dif_crc_fn);
+	return 0;
 }
 
-static void sd_dif_type3_generate_ip(struct blk_integrity_exchg *bix)
+static int sd_dif_type3_generate_ip(struct blk_integrity_iter *iter)
 {
-	sd_dif_type3_generate(bix, sd_dif_ip_fn);
+	sd_dif_type3_generate(iter, sd_dif_ip_fn);
+	return 0;
 }
 
-static int sd_dif_type3_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
+static int sd_dif_type3_verify(struct blk_integrity_iter *iter, csum_fn *fn)
 {
-	void *buf = bix->data_buf;
-	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t seed = bix->seed;
+	void *buf = iter->data_buf;
+	struct sd_dif_tuple *sdt = iter->prot_buf;
+	sector_t seed = iter->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
 			return 0;
 
-		csum = fn(buf, bix->interval);
+		csum = fn(buf, iter->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
-			       "(rcvd %04x, data %04x)\n", bix->disk_name,
+			       "(rcvd %04x, data %04x)\n", iter->disk_name,
 			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->interval;
+		buf += iter->interval;
 		seed++;
 	}
 
 	return 0;
 }
 
-static int sd_dif_type3_verify_crc(struct blk_integrity_exchg *bix)
+static int sd_dif_type3_verify_crc(struct blk_integrity_iter *iter)
 {
-	return sd_dif_type3_verify(bix, sd_dif_crc_fn);
+	return sd_dif_type3_verify(iter, sd_dif_crc_fn);
 }
 
-static int sd_dif_type3_verify_ip(struct blk_integrity_exchg *bix)
+static int sd_dif_type3_verify_ip(struct blk_integrity_iter *iter)
 {
-	return sd_dif_type3_verify(bix, sd_dif_ip_fn);
+	return sd_dif_type3_verify(iter, sd_dif_ip_fn);
 }
 
 static struct blk_integrity dif_type3_integrity_crc = {
@@ -310,6 +314,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 	phys = hw_sector & 0xffffffff;
 
 	__rq_for_each_bio(bio, rq) {
+		struct bio_integrity_payload *bip = bio_integrity(bio);
 		struct bio_vec iv;
 		struct bvec_iter iter;
 		unsigned int j;
@@ -318,9 +323,9 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 		if (bio_flagged(bio, BIO_MAPPED_INTEGRITY))
 			break;
 
-		virt = bio_integrity(bio)->bip_iter.bi_sector & 0xffffffff;
+		virt = bip_get_seed(bip) & 0xffffffff;
 
-		bip_for_each_vec(iv, bio_integrity(bio), iter) {
+		bip_for_each_vec(iv, bip, iter) {
 			sdt = kmap_atomic(iv.bv_page)
 				+ iv.bv_offset;
 
@@ -366,12 +371,13 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 		phys >>= 3;
 
 	__rq_for_each_bio(bio, scmd->request) {
+		struct bio_integrity_payload *bip = bio_integrity(bio);
 		struct bio_vec iv;
 		struct bvec_iter iter;
 
-		virt = bio_integrity(bio)->bip_iter.bi_sector & 0xffffffff;
+		virt = bip_get_seed(bip) & 0xffffffff;
 
-		bip_for_each_vec(iv, bio_integrity(bio), iter) {
+		bip_for_each_vec(iv, bip, iter) {
 			sdt = kmap_atomic(iv.bv_page)
 				+ iv.bv_offset;
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 448d8c052cb7..3fd36660fd10 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -322,6 +322,18 @@ struct bio_integrity_payload {
 	struct bio_vec		*bip_vec;
 	struct bio_vec		bip_inline_vecs[0];/* embedded bvec array */
 };
+
+static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
+{
+	return bip->bip_iter.bi_sector;
+}
+
+static inline void bip_set_seed(struct bio_integrity_payload *bip,
+				sector_t seed)
+{
+	bip->bip_iter.bi_sector = seed;
+}
+
 #endif /* CONFIG_BLK_DEV_INTEGRITY */
 
 extern void bio_trim(struct bio *bio, int offset, int size);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 41ea989b1d0d..5a7848f825a1 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,7 +1467,7 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 #define INTEGRITY_FLAG_READ	2	/* verify data integrity on read */
 #define INTEGRITY_FLAG_WRITE	4	/* generate data integrity on write */
 
-struct blk_integrity_exchg {
+struct blk_integrity_iter {
 	void			*prot_buf;
 	void			*data_buf;
 	sector_t		seed;
@@ -1476,12 +1476,11 @@ struct blk_integrity_exchg {
 	const char		*disk_name;
 };
 
-typedef void (integrity_gen_fn) (struct blk_integrity_exchg *);
-typedef int (integrity_vrfy_fn) (struct blk_integrity_exchg *);
+typedef int (integrity_processing_fn) (struct blk_integrity_iter *);
 
 struct blk_integrity {
-	integrity_gen_fn	*generate_fn;
-	integrity_vrfy_fn	*verify_fn;
+	integrity_processing_fn	*generate_fn;
+	integrity_processing_fn	*verify_fn;
 
 	unsigned short		flags;
 	unsigned short		tuple_size;
-- 
1.9.3


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

* [PATCH 08/14] block: Add prefix to block integrity profile flags
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (6 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 07/14] block: Clean up the code used to generate and verify integrity metadata Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 09/14] block: Add a disk flag to block integrity profile Martin K. Petersen
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

Add a BLK_ prefix to the integrity profile flags. Also rename the flags
to be more consistent with the generate/verify terminology in the rest
of the integrity code.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c  |  4 ++--
 block/blk-integrity.c  | 43 ++++++++++++++++++++++---------------------
 include/linux/blkdev.h |  6 ++++--
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 5ef996e8063f..c6bc2faf2af7 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -173,11 +173,11 @@ bool bio_integrity_enabled(struct bio *bio)
 		return false;
 
 	if (bio_data_dir(bio) == READ && bi->verify_fn != NULL &&
-	    (bi->flags & INTEGRITY_FLAG_READ))
+	    (bi->flags & BLK_INTEGRITY_VERIFY))
 		return true;
 
 	if (bio_data_dir(bio) == WRITE && bi->generate_fn != NULL &&
-	    (bi->flags & INTEGRITY_FLAG_WRITE))
+	    (bi->flags & BLK_INTEGRITY_GENERATE))
 		return true;
 
 	return false;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 3a83a7d08177..a7436ccc936b 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -269,42 +269,42 @@ static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page)
 		return sprintf(page, "0\n");
 }
 
-static ssize_t integrity_read_store(struct blk_integrity *bi,
-				    const char *page, size_t count)
+static ssize_t integrity_verify_store(struct blk_integrity *bi,
+				      const char *page, size_t count)
 {
 	char *p = (char *) page;
 	unsigned long val = simple_strtoul(p, &p, 10);
 
 	if (val)
-		bi->flags |= INTEGRITY_FLAG_READ;
+		bi->flags |= BLK_INTEGRITY_VERIFY;
 	else
-		bi->flags &= ~INTEGRITY_FLAG_READ;
+		bi->flags &= ~BLK_INTEGRITY_VERIFY;
 
 	return count;
 }
 
-static ssize_t integrity_read_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page)
 {
-	return sprintf(page, "%d\n", (bi->flags & INTEGRITY_FLAG_READ) != 0);
+	return sprintf(page, "%d\n", (bi->flags & BLK_INTEGRITY_VERIFY) != 0);
 }
 
-static ssize_t integrity_write_store(struct blk_integrity *bi,
-				     const char *page, size_t count)
+static ssize_t integrity_generate_store(struct blk_integrity *bi,
+					const char *page, size_t count)
 {
 	char *p = (char *) page;
 	unsigned long val = simple_strtoul(p, &p, 10);
 
 	if (val)
-		bi->flags |= INTEGRITY_FLAG_WRITE;
+		bi->flags |= BLK_INTEGRITY_GENERATE;
 	else
-		bi->flags &= ~INTEGRITY_FLAG_WRITE;
+		bi->flags &= ~BLK_INTEGRITY_GENERATE;
 
 	return count;
 }
 
-static ssize_t integrity_write_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page)
 {
-	return sprintf(page, "%d\n", (bi->flags & INTEGRITY_FLAG_WRITE) != 0);
+	return sprintf(page, "%d\n", (bi->flags & BLK_INTEGRITY_GENERATE) != 0);
 }
 
 static struct integrity_sysfs_entry integrity_format_entry = {
@@ -317,23 +317,23 @@ static struct integrity_sysfs_entry integrity_tag_size_entry = {
 	.show = integrity_tag_size_show,
 };
 
-static struct integrity_sysfs_entry integrity_read_entry = {
+static struct integrity_sysfs_entry integrity_verify_entry = {
 	.attr = { .name = "read_verify", .mode = S_IRUGO | S_IWUSR },
-	.show = integrity_read_show,
-	.store = integrity_read_store,
+	.show = integrity_verify_show,
+	.store = integrity_verify_store,
 };
 
-static struct integrity_sysfs_entry integrity_write_entry = {
+static struct integrity_sysfs_entry integrity_generate_entry = {
 	.attr = { .name = "write_generate", .mode = S_IRUGO | S_IWUSR },
-	.show = integrity_write_show,
-	.store = integrity_write_store,
+	.show = integrity_generate_show,
+	.store = integrity_generate_store,
 };
 
 static struct attribute *integrity_attrs[] = {
 	&integrity_format_entry.attr,
 	&integrity_tag_size_entry.attr,
-	&integrity_read_entry.attr,
-	&integrity_write_entry.attr,
+	&integrity_verify_entry.attr,
+	&integrity_generate_entry.attr,
 	NULL,
 };
 
@@ -406,7 +406,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 
 		kobject_uevent(&bi->kobj, KOBJ_ADD);
 
-		bi->flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
+		bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
 		bi->interval = queue_logical_block_size(disk->queue);
 		disk->integrity = bi;
 	} else
@@ -419,6 +419,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 		bi->verify_fn = template->verify_fn;
 		bi->tuple_size = template->tuple_size;
 		bi->tag_size = template->tag_size;
+		bi->flags |= template->flags;
 	} else
 		bi->name = bi_unsupported_name;
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 5a7848f825a1..54cbd96fe94a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1464,8 +1464,10 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 
-#define INTEGRITY_FLAG_READ	2	/* verify data integrity on read */
-#define INTEGRITY_FLAG_WRITE	4	/* generate data integrity on write */
+enum blk_integrity_flags {
+	BLK_INTEGRITY_VERIFY		= 1 << 0,
+	BLK_INTEGRITY_GENERATE		= 1 << 1,
+};
 
 struct blk_integrity_iter {
 	void			*prot_buf;
-- 
1.9.3


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

* [PATCH 09/14] block: Add a disk flag to block integrity profile
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (7 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 08/14] block: Add prefix to block integrity profile flags Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 10/14] block: Relocate bio integrity flags Martin K. Petersen
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

So far we have relied on the app tag size to determine whether a disk
has been formatted with T10 protection information or not. However, not
all target devices provide application tag storage.

Add a flag to the block integrity profile that indicates whether the
disk has been formatted with protection information.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
---
 Documentation/ABI/testing/sysfs-block |  8 ++++++++
 block/blk-integrity.c                 | 12 ++++++++++++
 drivers/scsi/sd_dif.c                 |  8 +++++++-
 include/linux/blkdev.h                |  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 279da08f7541..8df003963d99 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -53,6 +53,14 @@ Description:
 		512 bytes of data.
 
 
+What:		/sys/block/<disk>/integrity/device_is_integrity_capable
+Date:		July 2014
+Contact:	Martin K. Petersen <martin.petersen@oracle.com>
+Description:
+		Indicates whether a storage device is capable of storing
+		integrity metadata. Set if the device is T10 PI-capable.
+
+
 What:		/sys/block/<disk>/integrity/write_generate
 Date:		June 2008
 Contact:	Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index a7436ccc936b..1c6ba442cd91 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -307,6 +307,12 @@ static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page)
 	return sprintf(page, "%d\n", (bi->flags & BLK_INTEGRITY_GENERATE) != 0);
 }
 
+static ssize_t integrity_device_show(struct blk_integrity *bi, char *page)
+{
+	return sprintf(page, "%u\n",
+		       (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE) != 0);
+}
+
 static struct integrity_sysfs_entry integrity_format_entry = {
 	.attr = { .name = "format", .mode = S_IRUGO },
 	.show = integrity_format_show,
@@ -329,11 +335,17 @@ static struct integrity_sysfs_entry integrity_generate_entry = {
 	.store = integrity_generate_store,
 };
 
+static struct integrity_sysfs_entry integrity_device_entry = {
+	.attr = { .name = "device_is_integrity_capable", .mode = S_IRUGO },
+	.show = integrity_device_show,
+};
+
 static struct attribute *integrity_attrs[] = {
 	&integrity_format_entry.attr,
 	&integrity_tag_size_entry.attr,
 	&integrity_verify_entry.attr,
 	&integrity_generate_entry.attr,
+	&integrity_device_entry.attr,
 	NULL,
 };
 
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 801c41851a01..1e971c6f8c2b 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -270,7 +270,13 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 		  "Enabling DIX %s protection\n", disk->integrity->name);
 
 	/* Signal to block layer that we support sector tagging */
-	if (dif && type && sdkp->ATO) {
+	if (dif && type) {
+
+		disk->integrity->flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+
+		if (!sdkp)
+			return;
+
 		if (type == SD_DIF_TYPE3_PROTECTION)
 			disk->integrity->tag_size = sizeof(u16) + sizeof(u32);
 		else
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 54cbd96fe94a..fc1ac3cab086 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,6 +1467,7 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 enum blk_integrity_flags {
 	BLK_INTEGRITY_VERIFY		= 1 << 0,
 	BLK_INTEGRITY_GENERATE		= 1 << 1,
+	BLK_INTEGRITY_DEVICE_CAPABLE	= 1 << 2,
 };
 
 struct blk_integrity_iter {
-- 
1.9.3


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

* [PATCH 10/14] block: Relocate bio integrity flags
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (8 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 09/14] block: Add a disk flag to block integrity profile Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 11/14] block: Integrity checksum flag Martin K. Petersen
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

Move flags affecting the integrity code out of the bio bi_flags and into
the block integrity payload.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c     | 4 ++--
 drivers/scsi/sd_dif.c     | 4 ++--
 include/linux/bio.h       | 9 ++++++++-
 include/linux/blk_types.h | 6 ++----
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c6bc2faf2af7..7cac9c34615c 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -100,7 +100,7 @@ void bio_integrity_free(struct bio *bio)
 	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct bio_set *bs = bio->bi_pool;
 
-	if (bip->bip_owns_buf)
+	if (bip->bip_flags & BIP_BLOCK_INTEGRITY)
 		kfree(page_address(bip->bip_vec->bv_page) +
 		      bip->bip_vec->bv_offset);
 
@@ -293,7 +293,7 @@ int bio_integrity_prep(struct bio *bio)
 		return -EIO;
 	}
 
-	bip->bip_owns_buf = 1;
+	bip->bip_flags |= BIP_BLOCK_INTEGRITY;
 	bip->bip_iter.bi_size = len;
 	bip_set_seed(bip, bio->bi_iter.bi_sector);
 
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 1e971c6f8c2b..4ce636fdc15f 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -326,7 +326,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 		unsigned int j;
 
 		/* Already remapped? */
-		if (bio_flagged(bio, BIO_MAPPED_INTEGRITY))
+		if (bip->bip_flags & BIP_MAPPED_INTEGRITY)
 			break;
 
 		virt = bip_get_seed(bip) & 0xffffffff;
@@ -347,7 +347,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 			kunmap_atomic(sdt);
 		}
 
-		bio->bi_flags |= (1 << BIO_MAPPED_INTEGRITY);
+		bip->bip_flags |= BIP_MAPPED_INTEGRITY;
 	}
 }
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 3fd36660fd10..b508cf69206d 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -315,7 +315,7 @@ struct bio_integrity_payload {
 	unsigned short		bip_slab;	/* slab the bip came from */
 	unsigned short		bip_vcnt;	/* # of integrity bio_vecs */
 	unsigned short		bip_max_vcnt;	/* integrity bio_vec slots */
-	unsigned		bip_owns_buf:1;	/* should free bip_buf */
+	unsigned short		bip_flags;	/* control flags */
 
 	struct work_struct	bip_work;	/* I/O completion */
 
@@ -323,6 +323,13 @@ struct bio_integrity_payload {
 	struct bio_vec		bip_inline_vecs[0];/* embedded bvec array */
 };
 
+enum bip_flags {
+	BIP_BLOCK_INTEGRITY	= 1 << 0, /* block layer owns integrity data */
+	BIP_MAPPED_INTEGRITY	= 1 << 1, /* ref tag has been remapped */
+	BIP_CTRL_NOCHECK	= 1 << 2, /* disable HBA integrity checking */
+	BIP_DISK_NOCHECK	= 1 << 3, /* disable disk integrity checking */
+};
+
 static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
 {
 	return bip->bip_iter.bi_sector;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 0b7084f7c136..c96939d488ed 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -120,10 +120,8 @@ struct bio {
 #define BIO_USER_MAPPED 6	/* contains user pages */
 #define BIO_EOPNOTSUPP	7	/* not supported */
 #define BIO_NULL_MAPPED 8	/* contains invalid user pages */
-#define BIO_FS_INTEGRITY 9	/* fs owns integrity data, not block layer */
-#define BIO_QUIET	10	/* Make BIO Quiet */
-#define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
-#define BIO_SNAP_STABLE	12	/* bio data must be snapshotted during write */
+#define BIO_QUIET	9	/* Make BIO Quiet */
+#define BIO_SNAP_STABLE	10	/* bio data must be snapshotted during write */
 
 /*
  * Flags starting here get preserved by bio_reset() - this includes
-- 
1.9.3


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

* [PATCH 11/14] block: Integrity checksum flag
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (9 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 10/14] block: Relocate bio integrity flags Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 12/14] block: Don't merge requests if integrity flags differ Martin K. Petersen
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

Make the choice of checksum a per-I/O property by introducing a flag
that can be inspected by the SCSI layer. There are several reasons for
this:

 1. It allows us to switch choice of checksum without unloading and
    reloading the HBA driver.

 2. During error recovery we need to be able to tell the HBA that
    checksums read from disk should not be verified and converted to IP
    checksums.

 3. For error injection purposes we need to be able to write a bad guard
    tag to storage. Since the storage device only supports T10 CRC we
    need to be able to disable IP checksum conversion on the HBA.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/bio-integrity.c  | 3 +++
 drivers/scsi/sd_dif.c  | 6 ++++--
 include/linux/bio.h    | 1 +
 include/linux/blkdev.h | 1 +
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 7cac9c34615c..b5327751fecc 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -297,6 +297,9 @@ int bio_integrity_prep(struct bio *bio)
 	bip->bip_iter.bi_size = len;
 	bip_set_seed(bip, bio->bi_iter.bi_sector);
 
+	if (bi->flags & BLK_INTEGRITY_IP_CHECKSUM)
+		bip->bip_flags |= BIP_IP_CHECKSUM;
+
 	/* Map it */
 	offset = offset_in_page(buf);
 	for (i = 0 ; i < nr_pages ; i++) {
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 4ce636fdc15f..2198abee619e 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -255,12 +255,14 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 		return;
 
 	/* Enable DMA of protection information */
-	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP)
+	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP) {
 		if (type == SD_DIF_TYPE3_PROTECTION)
 			blk_integrity_register(disk, &dif_type3_integrity_ip);
 		else
 			blk_integrity_register(disk, &dif_type1_integrity_ip);
-	else
+
+		disk->integrity->flags |= BLK_INTEGRITY_IP_CHECKSUM;
+	} else
 		if (type == SD_DIF_TYPE3_PROTECTION)
 			blk_integrity_register(disk, &dif_type3_integrity_crc);
 		else
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b508cf69206d..14bff3fe56d4 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -328,6 +328,7 @@ enum bip_flags {
 	BIP_MAPPED_INTEGRITY	= 1 << 1, /* ref tag has been remapped */
 	BIP_CTRL_NOCHECK	= 1 << 2, /* disable HBA integrity checking */
 	BIP_DISK_NOCHECK	= 1 << 3, /* disable disk integrity checking */
+	BIP_IP_CHECKSUM		= 1 << 4, /* IP checksum */
 };
 
 static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index fc1ac3cab086..cbc3608d08c1 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1468,6 +1468,7 @@ enum blk_integrity_flags {
 	BLK_INTEGRITY_VERIFY		= 1 << 0,
 	BLK_INTEGRITY_GENERATE		= 1 << 1,
 	BLK_INTEGRITY_DEVICE_CAPABLE	= 1 << 2,
+	BLK_INTEGRITY_IP_CHECKSUM	= 1 << 3,
 };
 
 struct blk_integrity_iter {
-- 
1.9.3


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

* [PATCH 12/14] block: Don't merge requests if integrity flags differ
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (10 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 11/14] block: Integrity checksum flag Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 13/14] block: Add T10 Protection Information functions Martin K. Petersen
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

We'd occasionally merge requests with conflicting integrity flags.
Introduce a merge helper which checks that the requests have compatible
integrity payloads.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
---
 block/blk-integrity.c  | 36 ++++++++++++++++++++++++++----------
 block/blk-merge.c      |  6 +++---
 include/linux/blkdev.h | 20 ++++++++++----------
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 1c6ba442cd91..79ffb4855af0 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -186,37 +186,53 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
 }
 EXPORT_SYMBOL(blk_integrity_compare);
 
-int blk_integrity_merge_rq(struct request_queue *q, struct request *req,
-			   struct request *next)
+bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
+			    struct request *next)
 {
-	if (blk_integrity_rq(req) != blk_integrity_rq(next))
-		return -1;
+	if (blk_integrity_rq(req) == 0 && blk_integrity_rq(next) == 0)
+		return true;
+
+	if (blk_integrity_rq(req) == 0 || blk_integrity_rq(next) == 0)
+		return false;
+
+	if (bio_integrity(req->bio)->bip_flags !=
+	    bio_integrity(next->bio)->bip_flags)
+		return false;
 
 	if (req->nr_integrity_segments + next->nr_integrity_segments >
 	    q->limits.max_integrity_segments)
-		return -1;
+		return false;
 
-	return 0;
+	return true;
 }
 EXPORT_SYMBOL(blk_integrity_merge_rq);
 
-int blk_integrity_merge_bio(struct request_queue *q, struct request *req,
-			    struct bio *bio)
+bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
+			     struct bio *bio)
 {
 	int nr_integrity_segs;
 	struct bio *next = bio->bi_next;
 
+	if (blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL)
+		return true;
+
+	if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
+		return false;
+
+	if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags)
+		return false;
+
 	bio->bi_next = NULL;
 	nr_integrity_segs = blk_rq_count_integrity_sg(q, bio);
 	bio->bi_next = next;
 
 	if (req->nr_integrity_segments + nr_integrity_segs >
 	    q->limits.max_integrity_segments)
-		return -1;
+		return false;
 
 	req->nr_integrity_segments += nr_integrity_segs;
 
-	return 0;
+	return true;
 }
 EXPORT_SYMBOL(blk_integrity_merge_bio);
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 54535831f1e1..20dcc7d0aa8f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -308,7 +308,7 @@ static inline int ll_new_hw_segment(struct request_queue *q,
 	if (req->nr_phys_segments + nr_phys_segs > queue_max_segments(q))
 		goto no_merge;
 
-	if (bio_integrity(bio) && blk_integrity_merge_bio(q, req, bio))
+	if (blk_integrity_merge_bio(q, req, bio) == false)
 		goto no_merge;
 
 	/*
@@ -405,7 +405,7 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
 	if (total_phys_segments > queue_max_segments(q))
 		return 0;
 
-	if (blk_integrity_rq(req) && blk_integrity_merge_rq(q, req, next))
+	if (blk_integrity_merge_rq(q, req, next) == false)
 		return 0;
 
 	/* Merge is OK... */
@@ -585,7 +585,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
 		return false;
 
 	/* only merge integrity protected bio into ditto rq */
-	if (bio_integrity(bio) != blk_integrity_rq(rq))
+	if (blk_integrity_merge_bio(rq->q, rq, bio) == false)
 		return false;
 
 	/* must be using the same buffer */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index cbc3608d08c1..e8b2b0062cb8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1503,10 +1503,10 @@ extern int blk_integrity_compare(struct gendisk *, struct gendisk *);
 extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
 				   struct scatterlist *);
 extern int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
-extern int blk_integrity_merge_rq(struct request_queue *, struct request *,
-				  struct request *);
-extern int blk_integrity_merge_bio(struct request_queue *, struct request *,
-				   struct bio *);
+extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
+				   struct request *);
+extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
+				    struct bio *);
 
 static inline
 struct blk_integrity *bdev_get_integrity(struct block_device *bdev)
@@ -1586,15 +1586,15 @@ static inline unsigned short queue_max_integrity_segments(struct request_queue *
 {
 	return 0;
 }
-static inline int blk_integrity_merge_rq(struct request_queue *rq,
-					 struct request *r1,
-					 struct request *r2)
+static inline bool blk_integrity_merge_rq(struct request_queue *rq,
+					  struct request *r1,
+					  struct request *r2)
 {
 	return 0;
 }
-static inline int blk_integrity_merge_bio(struct request_queue *rq,
-					  struct request *r,
-					  struct bio *b)
+static inline bool blk_integrity_merge_bio(struct request_queue *rq,
+					   struct request *r,
+					   struct bio *b)
 {
 	return 0;
 }
-- 
1.9.3


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

* [PATCH 13/14] block: Add T10 Protection Information functions
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (11 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 12/14] block: Don't merge requests if integrity flags differ Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-28 19:31 ` [PATCH 14/14] sd: Honor block layer integrity handling flags Martin K. Petersen
  2014-08-28 20:10 ` Block/SCSI data integrity update v3 Jens Axboe
  14 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

The T10 Protection Information format is also used by some devices that
do not go through the SCSI layer (virtual block devices, NVMe). Relocate
the relevant functions to a block layer library that can be used without
involving SCSI.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/Kconfig              |   1 +
 block/Makefile             |   4 +-
 block/t10-pi.c             | 164 ++++++++++++++++++++++++++++++
 drivers/scsi/Kconfig       |   1 -
 drivers/scsi/sd_dif.c      | 241 ++++-----------------------------------------
 include/linux/crc-t10dif.h |   5 +-
 include/linux/t10-pi.h     |  60 +++++++++++
 7 files changed, 250 insertions(+), 226 deletions(-)
 create mode 100644 block/t10-pi.c
 create mode 100644 include/linux/t10-pi.h

diff --git a/block/Kconfig b/block/Kconfig
index 2429515c05c2..1427106f30c9 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -77,6 +77,7 @@ config BLK_DEV_BSGLIB
 
 config BLK_DEV_INTEGRITY
 	bool "Block layer data integrity support"
+	select CRC_T10_PI if BLK_DEV_INTEGRITY
 	---help---
 	Some storage devices allow extra information to be
 	stored/retrieved to help protect the data.  The block layer
diff --git a/block/Makefile b/block/Makefile
index a2ce6ac935ec..00ecc97629db 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -20,6 +20,6 @@ obj-$(CONFIG_IOSCHED_DEADLINE)	+= deadline-iosched.o
 obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched.o
 
 obj-$(CONFIG_BLOCK_COMPAT)	+= compat_ioctl.o
-obj-$(CONFIG_BLK_DEV_INTEGRITY)	+= blk-integrity.o
 obj-$(CONFIG_BLK_CMDLINE_PARSER)	+= cmdline-parser.o
-obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o
+obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
+
diff --git a/block/t10-pi.c b/block/t10-pi.c
new file mode 100644
index 000000000000..f45da5d55bd8
--- /dev/null
+++ b/block/t10-pi.c
@@ -0,0 +1,164 @@
+/*
+ * t10_pi.c - Functions for generating and verifying T10 Protection
+ *	      Information.
+ *
+ * Copyright (C) 2007, 2008, 2014 Oracle Corporation
+ * Written by: Martin K. Petersen <martin.petersen@oracle.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING.  If not, write to
+ * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139,
+ * USA.
+ *
+ */
+
+#include <linux/t10-pi.h>
+#include <linux/blkdev.h>
+#include <linux/crc-t10dif.h>
+#include <net/checksum.h>
+
+typedef __u16 (csum_fn) (void *, unsigned int);
+
+static __u16 t10_pi_crc_fn(void *data, unsigned int len)
+{
+	return cpu_to_be16(crc_t10dif(data, len));
+}
+
+static __u16 t10_pi_ip_fn(void *data, unsigned int len)
+{
+	return ip_compute_csum(data, len);
+}
+
+/*
+ * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
+ * 16 bit app tag, 32 bit reference tag. Type 3 does not define the ref
+ * tag.
+ */
+static int t10_pi_generate(struct blk_integrity_iter *iter, csum_fn *fn,
+			   unsigned int type)
+{
+	unsigned int i;
+
+	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->app_tag = 0;
+
+		if (type == 1)
+			pi->ref_tag = cpu_to_be32(iter->seed & 0xffffffff);
+		else
+			pi->ref_tag = 0;
+
+		iter->data_buf += iter->interval;
+		iter->prot_buf += sizeof(struct t10_pi_tuple);
+		iter->seed++;
+	}
+
+	return 0;
+}
+
+static int t10_pi_verify(struct blk_integrity_iter *iter, csum_fn *fn,
+				unsigned int type)
+{
+	unsigned int i;
+
+	for (i = 0 ; i < iter->data_size ; i += iter->interval) {
+		struct t10_pi_tuple *pi = iter->prot_buf;
+		__u16 csum;
+
+		switch (type) {
+		case 1:
+		case 2:
+			if (pi->app_tag == 0xffff)
+				goto next;
+
+			if (be32_to_cpu(pi->ref_tag) !=
+			    (iter->seed & 0xffffffff)) {
+				pr_err("%s: ref tag error at location %lu " \
+				       "(rcvd %u)\n", iter->disk_name,
+				       iter->seed, be32_to_cpu(pi->ref_tag));
+				return -EILSEQ;
+			}
+			break;
+		case 3:
+			if (pi->app_tag == 0xffff && pi->ref_tag == 0xffffffff)
+				goto next;
+			break;
+		}
+
+		csum = fn(iter->data_buf, iter->interval);
+
+		if (pi->guard_tag != csum) {
+			pr_err("%s: guard tag error at location %lu " \
+			       "(rcvd %04x, data %04x)\n", iter->disk_name,
+			       (unsigned long)iter->seed,
+			       be16_to_cpu(pi->guard_tag), be16_to_cpu(csum));
+			return -EILSEQ;
+		}
+
+next:
+		iter->data_buf += iter->interval;
+		iter->prot_buf += sizeof(struct t10_pi_tuple);
+		iter->seed++;
+	}
+
+	return 0;
+}
+
+int t10_pi_type1_generate_crc(struct blk_integrity_iter *iter)
+{
+	return t10_pi_generate(iter, t10_pi_crc_fn, 1);
+}
+EXPORT_SYMBOL(t10_pi_type1_generate_crc);
+
+int t10_pi_type1_generate_ip(struct blk_integrity_iter *iter)
+{
+	return t10_pi_generate(iter, t10_pi_ip_fn, 1);
+}
+EXPORT_SYMBOL(t10_pi_type1_generate_ip);
+
+int t10_pi_type1_verify_crc(struct blk_integrity_iter *iter)
+{
+	return t10_pi_verify(iter, t10_pi_crc_fn, 1);
+}
+EXPORT_SYMBOL(t10_pi_type1_verify_crc);
+
+int t10_pi_type1_verify_ip(struct blk_integrity_iter *iter)
+{
+	return t10_pi_verify(iter, t10_pi_ip_fn, 1);
+}
+EXPORT_SYMBOL(t10_pi_type1_verify_ip);
+
+int t10_pi_type3_generate_crc(struct blk_integrity_iter *iter)
+{
+	return t10_pi_generate(iter, t10_pi_crc_fn, 3);
+}
+EXPORT_SYMBOL(t10_pi_type3_generate_crc);
+
+int t10_pi_type3_generate_ip(struct blk_integrity_iter *iter)
+{
+	return t10_pi_generate(iter, t10_pi_ip_fn, 3);
+}
+EXPORT_SYMBOL(t10_pi_type3_generate_ip);
+
+int t10_pi_type3_verify_crc(struct blk_integrity_iter *iter)
+{
+	return t10_pi_verify(iter, t10_pi_crc_fn, 3);
+}
+EXPORT_SYMBOL(t10_pi_type3_verify_crc);
+
+int t10_pi_type3_verify_ip(struct blk_integrity_iter *iter)
+{
+	return t10_pi_verify(iter, t10_pi_ip_fn, 3);
+}
+EXPORT_SYMBOL(t10_pi_type3_verify_ip);
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 18a3358eb1d4..9ece13f922a6 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -62,7 +62,6 @@ comment "SCSI support type (disk, tape, CD-ROM)"
 config BLK_DEV_SD
 	tristate "SCSI disk support"
 	depends on SCSI
-	select CRC_T10DIF if BLK_DEV_INTEGRITY
 	---help---
 	  If you want to use SCSI hard disks, Fibre Channel disks,
 	  Serial ATA (SATA) or Parallel ATA (PATA) hard disks,
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 2198abee619e..b7eaeadc18f9 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -21,7 +21,7 @@
  */
 
 #include <linux/blkdev.h>
-#include <linux/crc-t10dif.h>
+#include <linux/t10-pi.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -33,207 +33,8 @@
 #include <scsi/scsi_ioctl.h>
 #include <scsi/scsicam.h>
 
-#include <net/checksum.h>
-
 #include "sd.h"
 
-typedef __u16 (csum_fn) (void *, unsigned int);
-
-static __u16 sd_dif_crc_fn(void *data, unsigned int len)
-{
-	return cpu_to_be16(crc_t10dif(data, len));
-}
-
-static __u16 sd_dif_ip_fn(void *data, unsigned int len)
-{
-	return ip_compute_csum(data, len);
-}
-
-/*
- * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
- * 16 bit app tag, 32 bit reference tag.
- */
-static void sd_dif_type1_generate(struct blk_integrity_iter *iter, csum_fn *fn)
-{
-	void *buf = iter->data_buf;
-	struct sd_dif_tuple *sdt = iter->prot_buf;
-	sector_t seed = iter->seed;
-	unsigned int i;
-
-	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
-		sdt->guard_tag = fn(buf, iter->interval);
-		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
-		sdt->app_tag = 0;
-
-		buf += iter->interval;
-		seed++;
-	}
-}
-
-static int sd_dif_type1_generate_crc(struct blk_integrity_iter *iter)
-{
-	sd_dif_type1_generate(iter, sd_dif_crc_fn);
-	return 0;
-}
-
-static int sd_dif_type1_generate_ip(struct blk_integrity_iter *iter)
-{
-	sd_dif_type1_generate(iter, sd_dif_ip_fn);
-	return 0;
-}
-
-static int sd_dif_type1_verify(struct blk_integrity_iter *iter, csum_fn *fn)
-{
-	void *buf = iter->data_buf;
-	struct sd_dif_tuple *sdt = iter->prot_buf;
-	sector_t seed = iter->seed;
-	unsigned int i;
-	__u16 csum;
-
-	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
-		/* Unwritten sectors */
-		if (sdt->app_tag == 0xffff)
-			return 0;
-
-		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
-			printk(KERN_ERR
-			       "%s: ref tag error on sector %lu (rcvd %u)\n",
-			       iter->disk_name, (unsigned long)seed,
-			       be32_to_cpu(sdt->ref_tag));
-			return -EIO;
-		}
-
-		csum = fn(buf, iter->interval);
-
-		if (sdt->guard_tag != csum) {
-			printk(KERN_ERR "%s: guard tag error on sector %lu " \
-			       "(rcvd %04x, data %04x)\n", iter->disk_name,
-			       (unsigned long)seed,
-			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
-			return -EIO;
-		}
-
-		buf += iter->interval;
-		seed++;
-	}
-
-	return 0;
-}
-
-static int sd_dif_type1_verify_crc(struct blk_integrity_iter *iter)
-{
-	return sd_dif_type1_verify(iter, sd_dif_crc_fn);
-}
-
-static int sd_dif_type1_verify_ip(struct blk_integrity_iter *iter)
-{
-	return sd_dif_type1_verify(iter, sd_dif_ip_fn);
-}
-
-static struct blk_integrity dif_type1_integrity_crc = {
-	.name			= "T10-DIF-TYPE1-CRC",
-	.generate_fn		= sd_dif_type1_generate_crc,
-	.verify_fn		= sd_dif_type1_verify_crc,
-	.tuple_size		= sizeof(struct sd_dif_tuple),
-	.tag_size		= 0,
-};
-
-static struct blk_integrity dif_type1_integrity_ip = {
-	.name			= "T10-DIF-TYPE1-IP",
-	.generate_fn		= sd_dif_type1_generate_ip,
-	.verify_fn		= sd_dif_type1_verify_ip,
-	.tuple_size		= sizeof(struct sd_dif_tuple),
-	.tag_size		= 0,
-};
-
-
-/*
- * Type 3 protection has a 16-bit guard tag and 16 + 32 bits of opaque
- * tag space.
- */
-static void sd_dif_type3_generate(struct blk_integrity_iter *iter, csum_fn *fn)
-{
-	void *buf = iter->data_buf;
-	struct sd_dif_tuple *sdt = iter->prot_buf;
-	unsigned int i;
-
-	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
-		sdt->guard_tag = fn(buf, iter->interval);
-		sdt->ref_tag = 0;
-		sdt->app_tag = 0;
-
-		buf += iter->interval;
-	}
-}
-
-static int sd_dif_type3_generate_crc(struct blk_integrity_iter *iter)
-{
-	sd_dif_type3_generate(iter, sd_dif_crc_fn);
-	return 0;
-}
-
-static int sd_dif_type3_generate_ip(struct blk_integrity_iter *iter)
-{
-	sd_dif_type3_generate(iter, sd_dif_ip_fn);
-	return 0;
-}
-
-static int sd_dif_type3_verify(struct blk_integrity_iter *iter, csum_fn *fn)
-{
-	void *buf = iter->data_buf;
-	struct sd_dif_tuple *sdt = iter->prot_buf;
-	sector_t seed = iter->seed;
-	unsigned int i;
-	__u16 csum;
-
-	for (i = 0 ; i < iter->data_size ; i += iter->interval, sdt++) {
-		/* Unwritten sectors */
-		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
-			return 0;
-
-		csum = fn(buf, iter->interval);
-
-		if (sdt->guard_tag != csum) {
-			printk(KERN_ERR "%s: guard tag error on sector %lu " \
-			       "(rcvd %04x, data %04x)\n", iter->disk_name,
-			       (unsigned long)seed,
-			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
-			return -EIO;
-		}
-
-		buf += iter->interval;
-		seed++;
-	}
-
-	return 0;
-}
-
-static int sd_dif_type3_verify_crc(struct blk_integrity_iter *iter)
-{
-	return sd_dif_type3_verify(iter, sd_dif_crc_fn);
-}
-
-static int sd_dif_type3_verify_ip(struct blk_integrity_iter *iter)
-{
-	return sd_dif_type3_verify(iter, sd_dif_ip_fn);
-}
-
-static struct blk_integrity dif_type3_integrity_crc = {
-	.name			= "T10-DIF-TYPE3-CRC",
-	.generate_fn		= sd_dif_type3_generate_crc,
-	.verify_fn		= sd_dif_type3_verify_crc,
-	.tuple_size		= sizeof(struct sd_dif_tuple),
-	.tag_size		= 0,
-};
-
-static struct blk_integrity dif_type3_integrity_ip = {
-	.name			= "T10-DIF-TYPE3-IP",
-	.generate_fn		= sd_dif_type3_generate_ip,
-	.verify_fn		= sd_dif_type3_verify_ip,
-	.tuple_size		= sizeof(struct sd_dif_tuple),
-	.tag_size		= 0,
-};
-
 /*
  * Configure exchange of protection information between OS and HBA.
  */
@@ -257,16 +58,16 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 	/* Enable DMA of protection information */
 	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP) {
 		if (type == SD_DIF_TYPE3_PROTECTION)
-			blk_integrity_register(disk, &dif_type3_integrity_ip);
+			blk_integrity_register(disk, &t10_pi_type3_ip);
 		else
-			blk_integrity_register(disk, &dif_type1_integrity_ip);
+			blk_integrity_register(disk, &t10_pi_type1_ip);
 
 		disk->integrity->flags |= BLK_INTEGRITY_IP_CHECKSUM;
 	} else
 		if (type == SD_DIF_TYPE3_PROTECTION)
-			blk_integrity_register(disk, &dif_type3_integrity_crc);
+			blk_integrity_register(disk, &t10_pi_type3_crc);
 		else
-			blk_integrity_register(disk, &dif_type1_integrity_crc);
+			blk_integrity_register(disk, &t10_pi_type1_crc);
 
 	sd_printk(KERN_NOTICE, sdkp,
 		  "Enabling DIX %s protection\n", disk->integrity->name);
@@ -308,10 +109,10 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 		    unsigned int sector_sz)
 {
-	const int tuple_sz = sizeof(struct sd_dif_tuple);
+	const int tuple_sz = sizeof(struct t10_pi_tuple);
 	struct bio *bio;
 	struct scsi_disk *sdkp;
-	struct sd_dif_tuple *sdt;
+	struct t10_pi_tuple *pi;
 	u32 phys, virt;
 
 	sdkp = rq->bio->bi_bdev->bd_disk->private_data;
@@ -334,19 +135,18 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 		virt = bip_get_seed(bip) & 0xffffffff;
 
 		bip_for_each_vec(iv, bip, iter) {
-			sdt = kmap_atomic(iv.bv_page)
-				+ iv.bv_offset;
+			pi = kmap_atomic(iv.bv_page) + iv.bv_offset;
 
-			for (j = 0; j < iv.bv_len; j += tuple_sz, sdt++) {
+			for (j = 0; j < iv.bv_len; j += tuple_sz, pi++) {
 
-				if (be32_to_cpu(sdt->ref_tag) == virt)
-					sdt->ref_tag = cpu_to_be32(phys);
+				if (be32_to_cpu(pi->ref_tag) == virt)
+					pi->ref_tag = cpu_to_be32(phys);
 
 				virt++;
 				phys++;
 			}
 
-			kunmap_atomic(sdt);
+			kunmap_atomic(pi);
 		}
 
 		bip->bip_flags |= BIP_MAPPED_INTEGRITY;
@@ -359,10 +159,10 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
  */
 void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 {
-	const int tuple_sz = sizeof(struct sd_dif_tuple);
+	const int tuple_sz = sizeof(struct t10_pi_tuple);
 	struct scsi_disk *sdkp;
 	struct bio *bio;
-	struct sd_dif_tuple *sdt;
+	struct t10_pi_tuple *pi;
 	unsigned int j, sectors, sector_sz;
 	u32 phys, virt;
 
@@ -386,25 +186,24 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 		virt = bip_get_seed(bip) & 0xffffffff;
 
 		bip_for_each_vec(iv, bip, iter) {
-			sdt = kmap_atomic(iv.bv_page)
-				+ iv.bv_offset;
+			pi = kmap_atomic(iv.bv_page) + iv.bv_offset;
 
-			for (j = 0; j < iv.bv_len; j += tuple_sz, sdt++) {
+			for (j = 0; j < iv.bv_len; j += tuple_sz, pi++) {
 
 				if (sectors == 0) {
-					kunmap_atomic(sdt);
+					kunmap_atomic(pi);
 					return;
 				}
 
-				if (be32_to_cpu(sdt->ref_tag) == phys)
-					sdt->ref_tag = cpu_to_be32(virt);
+				if (be32_to_cpu(pi->ref_tag) == phys)
+					pi->ref_tag = cpu_to_be32(virt);
 
 				virt++;
 				phys++;
 				sectors--;
 			}
 
-			kunmap_atomic(sdt);
+			kunmap_atomic(pi);
 		}
 	}
 }
diff --git a/include/linux/crc-t10dif.h b/include/linux/crc-t10dif.h
index b3cb71f0d3b0..cf53d0773ce3 100644
--- a/include/linux/crc-t10dif.h
+++ b/include/linux/crc-t10dif.h
@@ -6,7 +6,8 @@
 #define CRC_T10DIF_DIGEST_SIZE 2
 #define CRC_T10DIF_BLOCK_SIZE 1
 
-__u16 crc_t10dif_generic(__u16 crc, const unsigned char *buffer, size_t len);
-__u16 crc_t10dif(unsigned char const *, size_t);
+extern __u16 crc_t10dif_generic(__u16 crc, const unsigned char *buffer,
+				size_t len);
+extern __u16 crc_t10dif(unsigned char const *, size_t);
 
 #endif
diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h
new file mode 100644
index 000000000000..70569a6f2e8c
--- /dev/null
+++ b/include/linux/t10-pi.h
@@ -0,0 +1,60 @@
+#ifndef _LINUX_T10_PI_H
+#define _LINUX_T10_PI_H
+
+#include <linux/types.h>
+#include <linux/blkdev.h>
+
+/*
+ * T10 Protection Information tuple.
+ */
+struct t10_pi_tuple {
+	__be16 guard_tag;	/* Checksum */
+	__be16 app_tag;		/* Opaque storage */
+	__be32 ref_tag;		/* Target LBA or indirect LBA */
+};
+
+extern int t10_pi_type1_generate_crc(struct blk_integrity_iter *);
+extern int t10_pi_type1_verify_crc(struct blk_integrity_iter *);
+
+extern int t10_pi_type1_generate_ip(struct blk_integrity_iter *);
+extern int t10_pi_type1_verify_ip(struct blk_integrity_iter *);
+
+extern int t10_pi_type3_generate_crc(struct blk_integrity_iter *);
+extern int t10_pi_type3_verify_crc(struct blk_integrity_iter *);
+
+extern int t10_pi_type3_generate_ip(struct blk_integrity_iter *);
+extern int t10_pi_type3_verify_ip(struct blk_integrity_iter *);
+
+struct blk_integrity t10_pi_type1_crc = {
+	.name			= "T10-DIF-TYPE1-CRC",
+	.generate_fn		= t10_pi_type1_generate_crc,
+	.verify_fn		= t10_pi_type1_verify_crc,
+	.tuple_size		= sizeof(struct t10_pi_tuple),
+	.tag_size		= 0,
+};
+
+struct blk_integrity t10_pi_type1_ip = {
+	.name			= "T10-DIF-TYPE1-IP",
+	.generate_fn		= t10_pi_type1_generate_ip,
+	.verify_fn		= t10_pi_type1_verify_ip,
+	.tuple_size		= sizeof(struct t10_pi_tuple),
+	.tag_size		= 0,
+};
+
+struct blk_integrity t10_pi_type3_crc = {
+	.name			= "T10-DIF-TYPE3-CRC",
+	.generate_fn		= t10_pi_type3_generate_crc,
+	.verify_fn		= t10_pi_type3_verify_crc,
+	.tuple_size		= sizeof(struct t10_pi_tuple),
+	.tag_size		= 0,
+};
+
+struct blk_integrity t10_pi_type3_ip = {
+	.name			= "T10-DIF-TYPE3-IP",
+	.generate_fn		= t10_pi_type3_generate_ip,
+	.verify_fn		= t10_pi_type3_verify_ip,
+	.tuple_size		= sizeof(struct t10_pi_tuple),
+	.tag_size		= 0,
+};
+
+#endif
-- 
1.9.3


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

* [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (12 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 13/14] block: Add T10 Protection Information functions Martin K. Petersen
@ 2014-08-28 19:31 ` Martin K. Petersen
  2014-08-31 21:17   ` Sagi Grimberg
  2014-08-28 20:10 ` Block/SCSI data integrity update v3 Jens Axboe
  14 siblings, 1 reply; 36+ messages in thread
From: Martin K. Petersen @ 2014-08-28 19:31 UTC (permalink / raw)
  To: linux-scsi, axboe; +Cc: hch, sagig, Martin K. Petersen

A set of flags introduced in the block layer enable better control over
how protection information is handled. These flags are useful for both
error injection and data recovery purposes. Checking can be enabled and
disabled for controller and disk, and the guard tag format is now a
per-I/O property.

Update sd_protect_op to communicate the relevant information to the
low-level device driver via a set of flags in scsi_cmnd.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c        | 73 ++++++++++++++++++++++++++++--------------------
 drivers/scsi/sd.h        | 66 +++++++++++++++++++++++++++++++++++++++++--
 drivers/scsi/sd_dif.c    | 23 ++++++---------
 include/scsi/scsi_cmnd.h | 36 +++++++++++++++++-------
 4 files changed, 142 insertions(+), 56 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2c2041ca4b70..53d049cc1a1e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -610,29 +610,44 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
 	mutex_unlock(&sd_ref_mutex);
 }
 
-static void sd_prot_op(struct scsi_cmnd *scmd, unsigned int dif)
-{
-	unsigned int prot_op = SCSI_PROT_NORMAL;
-	unsigned int dix = scsi_prot_sg_count(scmd);
-
-	if (scmd->sc_data_direction == DMA_FROM_DEVICE) {
-		if (dif && dix)
-			prot_op = SCSI_PROT_READ_PASS;
-		else if (dif && !dix)
-			prot_op = SCSI_PROT_READ_STRIP;
-		else if (!dif && dix)
-			prot_op = SCSI_PROT_READ_INSERT;
-	} else {
-		if (dif && dix)
-			prot_op = SCSI_PROT_WRITE_PASS;
-		else if (dif && !dix)
-			prot_op = SCSI_PROT_WRITE_INSERT;
-		else if (!dif && dix)
-			prot_op = SCSI_PROT_WRITE_STRIP;
+
+
+static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
+					   unsigned int dix, unsigned int dif)
+{
+	struct bio_integrity_payload *bip = bio_integrity(scmd->request->bio);
+	unsigned int prot_op = sd_prot_op(rq_data_dir(scmd->request), dix, dif);
+	unsigned int protect = 0;
+
+	if (dix) {				/* DIX Type 0, 1, 2, 3 */
+		if (bip->bip_flags & BIP_IP_CHECKSUM)
+			scmd->prot_flags |= SCSI_PROT_IP_CHECKSUM;
+
+		if ((bip->bip_flags & BIP_CTRL_NOCHECK) == 0)
+			scmd->prot_flags |= SCSI_PROT_GUARD_CHECK;
+	}
+
+	if (dif != SD_DIF_TYPE3_PROTECTION) {	/* DIX/DIF Type 0, 1, 2 */
+		scmd->prot_flags |= SCSI_PROT_REF_INCREMENT;
+
+		if ((bip && bip->bip_flags & BIP_CTRL_NOCHECK) == 0)
+			scmd->prot_flags |= SCSI_PROT_REF_CHECK;
+	}
+
+	if (dif) {				/* DIX/DIF Type 1, 2, 3 */
+		scmd->prot_flags |= SCSI_PROT_TRANSFER_PI;
+
+		if (bip && bip->bip_flags & BIP_DISK_NOCHECK)
+			protect = 3 << 5;	/* Disable target PI checking */
+		else
+			protect = 1 << 5;	/* Enable target PI checking */
 	}
 
 	scsi_set_prot_op(scmd, prot_op);
 	scsi_set_prot_type(scmd, dif);
+	scmd->prot_flags &= sd_prot_flag_mask(prot_op);
+
+	return protect;
 }
 
 static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
@@ -893,7 +908,8 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	sector_t block = blk_rq_pos(rq);
 	sector_t threshold;
 	unsigned int this_count = blk_rq_sectors(rq);
-	int ret, host_dif;
+	unsigned int dif, dix;
+	int ret;
 	unsigned char protect;
 
 	ret = scsi_init_io(SCpnt, GFP_ATOMIC);
@@ -995,7 +1011,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 		SCpnt->cmnd[0] = WRITE_6;
 
 		if (blk_integrity_rq(rq))
-			sd_dif_prepare(rq, block, sdp->sector_size);
+			sd_dif_prepare(SCpnt);
 
 	} else if (rq_data_dir(rq) == READ) {
 		SCpnt->cmnd[0] = READ_6;
@@ -1010,14 +1026,15 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 					"writing" : "reading", this_count,
 					blk_rq_sectors(rq)));
 
-	/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
-	host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
-	if (host_dif)
-		protect = 1 << 5;
+	dix = scsi_prot_sg_count(SCpnt);
+	dif = scsi_host_dif_capable(SCpnt->device->host, sdkp->protection_type);
+
+	if (dif || dix)
+		protect = sd_setup_protect_cmnd(SCpnt, dix, dif);
 	else
 		protect = 0;
 
-	if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+	if (protect && sdkp->protection_type == SD_DIF_TYPE2_PROTECTION) {
 		SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
 
 		if (unlikely(SCpnt->cmnd == NULL)) {
@@ -1102,10 +1119,6 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	}
 	SCpnt->sdb.length = this_count * sdp->sector_size;
 
-	/* If DIF or DIX is enabled, tell HBA how to handle request */
-	if (host_dif || scsi_prot_sg_count(SCpnt))
-		sd_prot_op(SCpnt, host_dif);
-
 	/*
 	 * We shouldn't disconnect in the middle of a sector, so with a dumb
 	 * host adapter, it's safe to assume that we can at least transfer
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 4c3ab8377fd3..467377884b63 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -167,6 +167,68 @@ enum sd_dif_target_protection_types {
 };
 
 /*
+ * Look up the DIX operation based on whether the command is read or
+ * write and whether dix and dif are enabled.
+ */
+static inline unsigned int sd_prot_op(bool write, bool dix, bool dif)
+{
+	/* Lookup table: bit 2 (write), bit 1 (dix), bit 0 (dif) */
+	const unsigned int ops[] = {	/* wrt dix dif */
+		SCSI_PROT_NORMAL,	/*  0	0   0  */
+		SCSI_PROT_READ_STRIP,	/*  0	0   1  */
+		SCSI_PROT_READ_INSERT,	/*  0	1   0  */
+		SCSI_PROT_READ_PASS,	/*  0	1   1  */
+		SCSI_PROT_NORMAL,	/*  1	0   0  */
+		SCSI_PROT_WRITE_INSERT, /*  1	0   1  */
+		SCSI_PROT_WRITE_STRIP,	/*  1	1   0  */
+		SCSI_PROT_WRITE_PASS,	/*  1	1   1  */
+	};
+
+	return ops[write << 2 | dix << 1 | dif];
+}
+
+/*
+ * Returns a mask of the protection flags that are valid for a given DIX
+ * operation.
+ */
+static inline unsigned int sd_prot_flag_mask(unsigned int prot_op)
+{
+	const unsigned int flag_mask[] = {
+		[SCSI_PROT_NORMAL]		= 0,
+
+		[SCSI_PROT_READ_STRIP]		= SCSI_PROT_TRANSFER_PI |
+						  SCSI_PROT_GUARD_CHECK |
+						  SCSI_PROT_REF_CHECK |
+						  SCSI_PROT_REF_INCREMENT,
+
+		[SCSI_PROT_READ_INSERT]		= SCSI_PROT_REF_INCREMENT |
+						  SCSI_PROT_IP_CHECKSUM,
+
+		[SCSI_PROT_READ_PASS]		= SCSI_PROT_TRANSFER_PI |
+						  SCSI_PROT_GUARD_CHECK |
+						  SCSI_PROT_REF_CHECK |
+						  SCSI_PROT_REF_INCREMENT |
+						  SCSI_PROT_IP_CHECKSUM,
+
+		[SCSI_PROT_WRITE_INSERT]	= SCSI_PROT_TRANSFER_PI |
+						  SCSI_PROT_REF_INCREMENT,
+
+		[SCSI_PROT_WRITE_STRIP]		= SCSI_PROT_GUARD_CHECK |
+						  SCSI_PROT_REF_CHECK |
+						  SCSI_PROT_REF_INCREMENT |
+						  SCSI_PROT_IP_CHECKSUM,
+
+		[SCSI_PROT_WRITE_PASS]		= SCSI_PROT_TRANSFER_PI |
+						  SCSI_PROT_GUARD_CHECK |
+						  SCSI_PROT_REF_CHECK |
+						  SCSI_PROT_REF_INCREMENT |
+						  SCSI_PROT_IP_CHECKSUM,
+	};
+
+	return flag_mask[prot_op];
+}
+
+/*
  * Data Integrity Field tuple.
  */
 struct sd_dif_tuple {
@@ -178,7 +240,7 @@ struct sd_dif_tuple {
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 
 extern void sd_dif_config_host(struct scsi_disk *);
-extern void sd_dif_prepare(struct request *rq, sector_t, unsigned int);
+extern void sd_dif_prepare(struct scsi_cmnd *scmd);
 extern void sd_dif_complete(struct scsi_cmnd *, unsigned int);
 
 #else /* CONFIG_BLK_DEV_INTEGRITY */
@@ -186,7 +248,7 @@ extern void sd_dif_complete(struct scsi_cmnd *, unsigned int);
 static inline void sd_dif_config_host(struct scsi_disk *disk)
 {
 }
-static inline int sd_dif_prepare(struct request *rq, sector_t s, unsigned int a)
+static inline int sd_dif_prepare(struct scsi_cmnd *scmd)
 {
 	return 0;
 }
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index b7eaeadc18f9..14c7d42a11c2 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -106,8 +106,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
  *
  * Type 3 does not have a reference tag so no remapping is required.
  */
-void sd_dif_prepare(struct request *rq, sector_t hw_sector,
-		    unsigned int sector_sz)
+void sd_dif_prepare(struct scsi_cmnd *scmd)
 {
 	const int tuple_sz = sizeof(struct t10_pi_tuple);
 	struct bio *bio;
@@ -115,14 +114,14 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
 	struct t10_pi_tuple *pi;
 	u32 phys, virt;
 
-	sdkp = rq->bio->bi_bdev->bd_disk->private_data;
+	sdkp = scsi_disk(scmd->request->rq_disk);
 
 	if (sdkp->protection_type == SD_DIF_TYPE3_PROTECTION)
 		return;
 
-	phys = hw_sector & 0xffffffff;
+	phys = scsi_prot_ref_tag(scmd);
 
-	__rq_for_each_bio(bio, rq) {
+	__rq_for_each_bio(bio, scmd->request) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
 		struct bio_vec iv;
 		struct bvec_iter iter;
@@ -163,7 +162,7 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 	struct scsi_disk *sdkp;
 	struct bio *bio;
 	struct t10_pi_tuple *pi;
-	unsigned int j, sectors, sector_sz;
+	unsigned int j, intervals;
 	u32 phys, virt;
 
 	sdkp = scsi_disk(scmd->request->rq_disk);
@@ -171,12 +170,8 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 	if (sdkp->protection_type == SD_DIF_TYPE3_PROTECTION || good_bytes == 0)
 		return;
 
-	sector_sz = scmd->device->sector_size;
-	sectors = good_bytes / sector_sz;
-
-	phys = blk_rq_pos(scmd->request) & 0xffffffff;
-	if (sector_sz == 4096)
-		phys >>= 3;
+	intervals = good_bytes / scsi_prot_interval(scmd);
+	phys = scsi_prot_ref_tag(scmd);
 
 	__rq_for_each_bio(bio, scmd->request) {
 		struct bio_integrity_payload *bip = bio_integrity(bio);
@@ -190,7 +185,7 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 
 			for (j = 0; j < iv.bv_len; j += tuple_sz, pi++) {
 
-				if (sectors == 0) {
+				if (intervals == 0) {
 					kunmap_atomic(pi);
 					return;
 				}
@@ -200,7 +195,7 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 
 				virt++;
 				phys++;
-				sectors--;
+				intervals--;
 			}
 
 			kunmap_atomic(pi);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 73f349044941..522a5f27f553 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -10,9 +10,10 @@
 #include <scsi/scsi_device.h>
 
 struct Scsi_Host;
-struct scsi_device;
 struct scsi_driver;
 
+#include <scsi/scsi_device.h>
+
 /*
  * MAX_COMMAND_SIZE is:
  * The longest fixed-length SCSI CDB as per the SCSI standard.
@@ -81,6 +82,7 @@ struct scsi_cmnd {
 
 	unsigned char prot_op;
 	unsigned char prot_type;
+	unsigned char prot_flags;
 
 	unsigned short cmd_len;
 	enum dma_data_direction sc_data_direction;
@@ -252,6 +254,14 @@ static inline unsigned char scsi_get_prot_op(struct scsi_cmnd *scmd)
 	return scmd->prot_op;
 }
 
+enum scsi_prot_flags {
+	SCSI_PROT_TRANSFER_PI		= 1 << 0,
+	SCSI_PROT_GUARD_CHECK		= 1 << 1,
+	SCSI_PROT_REF_CHECK		= 1 << 2,
+	SCSI_PROT_REF_INCREMENT		= 1 << 3,
+	SCSI_PROT_IP_CHECKSUM		= 1 << 4,
+};
+
 /*
  * The controller usually does not know anything about the target it
  * is communicating with.  However, when DIX is enabled the controller
@@ -280,6 +290,17 @@ static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
 	return blk_rq_pos(scmd->request);
 }
 
+static inline unsigned int scsi_prot_interval(struct scsi_cmnd *scmd)
+{
+	return scmd->device->sector_size;
+}
+
+static inline u32 scsi_prot_ref_tag(struct scsi_cmnd *scmd)
+{
+	return blk_rq_pos(scmd->request) >>
+		(ilog2(scsi_prot_interval(scmd)) - 9) & 0xffffffff;
+}
+
 static inline unsigned scsi_prot_sg_count(struct scsi_cmnd *cmd)
 {
 	return cmd->prot_sdb ? cmd->prot_sdb->table.nents : 0;
@@ -316,17 +337,12 @@ static inline void set_driver_byte(struct scsi_cmnd *cmd, char status)
 static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
 {
 	unsigned int xfer_len = scsi_out(scmd)->length;
-	unsigned int prot_op = scsi_get_prot_op(scmd);
-	unsigned int sector_size = scmd->device->sector_size;
+	unsigned int prot_interval = scsi_prot_interval(scmd);
 
-	switch (prot_op) {
-	case SCSI_PROT_NORMAL:
-	case SCSI_PROT_WRITE_STRIP:
-	case SCSI_PROT_READ_INSERT:
-		return xfer_len;
-	}
+	if (scmd->prot_flags & SCSI_PROT_TRANSFER_PI)
+		xfer_len += (xfer_len >> ilog2(prot_interval)) * 8;
 
-	return xfer_len + (xfer_len >> ilog2(sector_size)) * 8;
+	return xfer_len;
 }
 
 #endif /* _SCSI_SCSI_CMND_H */
-- 
1.9.3


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

* Re: Block/SCSI data integrity update v3
  2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
                   ` (13 preceding siblings ...)
  2014-08-28 19:31 ` [PATCH 14/14] sd: Honor block layer integrity handling flags Martin K. Petersen
@ 2014-08-28 20:10 ` Jens Axboe
  2014-08-29  0:43   ` Christoph Hellwig
  2014-09-07 16:29   ` Christoph Hellwig
  14 siblings, 2 replies; 36+ messages in thread
From: Jens Axboe @ 2014-08-28 20:10 UTC (permalink / raw)
  To: Martin K. Petersen, linux-scsi; +Cc: hch, sagig

On 08/28/2014 01:31 PM, Martin K. Petersen wrote:
> This is the data integrity patch series originally submitted for 3.16
> and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
> than that there are no changes from v2.

Thanks, picked up. I did not apply 14/14 since that should go through
the SCSI tree. I can carry it if need be, but it'd be nice to have a
SCSI signoff on it then.

-- 
Jens Axboe


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

* Re: Block/SCSI data integrity update v3
  2014-08-28 20:10 ` Block/SCSI data integrity update v3 Jens Axboe
@ 2014-08-29  0:43   ` Christoph Hellwig
  2014-09-07 16:29   ` Christoph Hellwig
  1 sibling, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2014-08-29  0:43 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Martin K. Petersen, linux-scsi, hch, sagig

On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote:
> On 08/28/2014 01:31 PM, Martin K. Petersen wrote:
> > This is the data integrity patch series originally submitted for 3.16
> > and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
> > than that there are no changes from v2.
> 
> Thanks, picked up. I did not apply 14/14 since that should go through
> the SCSI tree. I can carry it if need be, but it'd be nice to have a
> SCSI signoff on it then.

I'd rather avoid introducing a dependency of the SCSI tree on the block
one, so I'd prefer if you merge the last one as well.  I'll try to find
some time to review it ASAP.

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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-08-28 19:31 ` [PATCH 14/14] sd: Honor block layer integrity handling flags Martin K. Petersen
@ 2014-08-31 21:17   ` Sagi Grimberg
  2014-09-11  0:07     ` Martin K. Petersen
  0 siblings, 1 reply; 36+ messages in thread
From: Sagi Grimberg @ 2014-08-31 21:17 UTC (permalink / raw)
  To: Martin K. Petersen, linux-scsi, axboe; +Cc: hch, sagig

On 8/28/2014 10:31 PM, Martin K. Petersen wrote:
> A set of flags introduced in the block layer enable better control over
> how protection information is handled. These flags are useful for both
> error injection and data recovery purposes. Checking can be enabled and
> disabled for controller and disk, and the guard tag format is now a
> per-I/O property.
>

Hi Martin,

Not sure why I didn't post a review for this one...
Anyway sorry, I hope it's not too late...
just two comments below.

> Update sd_protect_op to communicate the relevant information to the
> low-level device driver via a set of flags in scsi_cmnd.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>   drivers/scsi/sd.c        | 73 ++++++++++++++++++++++++++++--------------------
>   drivers/scsi/sd.h        | 66 +++++++++++++++++++++++++++++++++++++++++--
>   drivers/scsi/sd_dif.c    | 23 ++++++---------
>   include/scsi/scsi_cmnd.h | 36 +++++++++++++++++-------
>   4 files changed, 142 insertions(+), 56 deletions(-)
>

<SNIP>

> +/*
> + * Returns a mask of the protection flags that are valid for a given DIX
> + * operation.
> + */
> +static inline unsigned int sd_prot_flag_mask(unsigned int prot_op)
> +{
> +	const unsigned int flag_mask[] = {
> +		[SCSI_PROT_NORMAL]		= 0,
> +
> +		[SCSI_PROT_READ_STRIP]		= SCSI_PROT_TRANSFER_PI |
> +						  SCSI_PROT_GUARD_CHECK |
> +						  SCSI_PROT_REF_CHECK |
> +						  SCSI_PROT_REF_INCREMENT,
> +
> +		[SCSI_PROT_READ_INSERT]		= SCSI_PROT_REF_INCREMENT |
> +						  SCSI_PROT_IP_CHECKSUM,
> +
> +		[SCSI_PROT_READ_PASS]		= SCSI_PROT_TRANSFER_PI |
> +						  SCSI_PROT_GUARD_CHECK |
> +						  SCSI_PROT_REF_CHECK |
> +						  SCSI_PROT_REF_INCREMENT |
> +						  SCSI_PROT_IP_CHECKSUM,
> +
> +		[SCSI_PROT_WRITE_INSERT]	= SCSI_PROT_TRANSFER_PI |
> +						  SCSI_PROT_REF_INCREMENT,
> +
> +		[SCSI_PROT_WRITE_STRIP]		= SCSI_PROT_GUARD_CHECK |
> +						  SCSI_PROT_REF_CHECK |
> +						  SCSI_PROT_REF_INCREMENT |
> +						  SCSI_PROT_IP_CHECKSUM,
> +
> +		[SCSI_PROT_WRITE_PASS]		= SCSI_PROT_TRANSFER_PI |
> +						  SCSI_PROT_GUARD_CHECK |
> +						  SCSI_PROT_REF_CHECK |
> +						  SCSI_PROT_REF_INCREMENT |
> +						  SCSI_PROT_IP_CHECKSUM,

A bit strange to me that you put REF_CHECK & REF_INCREMENT flag
depending on the prot_op while it really depends on the prot_type.
It is &'nd with preset scmnd->prot_flags anyways.. so no worries...

> +	};
> +
> +	return flag_mask[prot_op];
> +}
> +

<SNIP>

> +/*
> +enum scsi_prot_flags {
> +	SCSI_PROT_TRANSFER_PI		= 1 << 0,
> +	SCSI_PROT_GUARD_CHECK		= 1 << 1,
> +	SCSI_PROT_REF_CHECK		= 1 << 2,
> +	SCSI_PROT_REF_INCREMENT		= 1 << 3,
> +	SCSI_PROT_IP_CHECKSUM		= 1 << 4,

I have a question here,
What are your plans with explicit ref/app escape flags?
Were these just left behind on this series?


Other than that, looks good.

Reviewed-by: Sagi Grimberg <sagig@mellanox.com>

Sagi.

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

* Re: Block/SCSI data integrity update v3
  2014-08-28 20:10 ` Block/SCSI data integrity update v3 Jens Axboe
  2014-08-29  0:43   ` Christoph Hellwig
@ 2014-09-07 16:29   ` Christoph Hellwig
  2014-09-08 15:50     ` Jens Axboe
  1 sibling, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2014-09-07 16:29 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Martin K. Petersen, linux-scsi, hch, sagig

On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote:
> On 08/28/2014 01:31 PM, Martin K. Petersen wrote:
> > This is the data integrity patch series originally submitted for 3.16
> > and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
> > than that there are no changes from v2.
> 
> Thanks, picked up. I did not apply 14/14 since that should go through
> the SCSI tree. I can carry it if need be, but it'd be nice to have a
> SCSI signoff on it then.

While Sagi found some minor nitpicks I think we could address them later
if needed, so

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

from me for picking it up through the block tree.

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

* Re: Block/SCSI data integrity update v3
  2014-09-07 16:29   ` Christoph Hellwig
@ 2014-09-08 15:50     ` Jens Axboe
  2014-09-11  0:23       ` Martin K. Petersen
  0 siblings, 1 reply; 36+ messages in thread
From: Jens Axboe @ 2014-09-08 15:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-scsi, hch, sagig

On 09/07/2014 10:29 AM, Christoph Hellwig wrote:
> On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote:
>> On 08/28/2014 01:31 PM, Martin K. Petersen wrote:
>>> This is the data integrity patch series originally submitted for 3.16
>>> and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
>>> than that there are no changes from v2.
>>
>> Thanks, picked up. I did not apply 14/14 since that should go through
>> the SCSI tree. I can carry it if need be, but it'd be nice to have a
>> SCSI signoff on it then.
> 
> While Sagi found some minor nitpicks I think we could address them later
> if needed, so
> 
> Acked-by: Christoph Hellwig <hch@lst.de>
> 
> from me for picking it up through the block tree.

Thanks, I'll do that once Martin respins the previous series (was pulled
due to multiple issues).

-- 
Jens Axboe


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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-08-31 21:17   ` Sagi Grimberg
@ 2014-09-11  0:07     ` Martin K. Petersen
  2014-09-11  7:09       ` Sagi Grimberg
  0 siblings, 1 reply; 36+ messages in thread
From: Martin K. Petersen @ 2014-09-11  0:07 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: Martin K. Petersen, linux-scsi, axboe, hch, sagig

>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:

[Back from vacation]

>> + [SCSI_PROT_WRITE_PASS] = SCSI_PROT_TRANSFER_PI |
>> + SCSI_PROT_GUARD_CHECK |
>> + SCSI_PROT_REF_CHECK |
>> + SCSI_PROT_REF_INCREMENT |
>> + SCSI_PROT_IP_CHECKSUM,

Sagi> A bit strange to me that you put REF_CHECK & REF_INCREMENT flag
Sagi> depending on the prot_op while it really depends on the prot_type.

It doesn't just depend on the prot_type. You need to be able to tell the
HBA to increment/test the ref tag even for a Type 0 device.

Sagi> I have a question here, What are your plans with explicit ref/app
Sagi> escape flags?  Were these just left behind on this series?

We don't yet have a need for them in Linux so I didn't define them.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Block/SCSI data integrity update v3
  2014-09-08 15:50     ` Jens Axboe
@ 2014-09-11  0:23       ` Martin K. Petersen
  2014-09-12 18:16         ` Jens Axboe
  0 siblings, 1 reply; 36+ messages in thread
From: Martin K. Petersen @ 2014-09-11  0:23 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, Martin K. Petersen, linux-scsi, hch, sagig

>>>>> "Jens" == Jens Axboe <axboe@fb.com> writes:

Jens> Thanks, I'll do that once Martin respins the previous series (was
Jens> pulled due to multiple issues).

I bumped bi_rw to u64 as discussed. Will get that into -next to see if
there's any fallout.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-09-11  0:07     ` Martin K. Petersen
@ 2014-09-11  7:09       ` Sagi Grimberg
  2014-09-12  1:35         ` Martin K. Petersen
  0 siblings, 1 reply; 36+ messages in thread
From: Sagi Grimberg @ 2014-09-11  7:09 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi, axboe, hch, sagig

On 9/11/2014 3:07 AM, Martin K. Petersen wrote:
>>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:
>
> [Back from vacation]

Hey Martin,

>
>>> + [SCSI_PROT_WRITE_PASS] = SCSI_PROT_TRANSFER_PI |
>>> + SCSI_PROT_GUARD_CHECK |
>>> + SCSI_PROT_REF_CHECK |
>>> + SCSI_PROT_REF_INCREMENT |
>>> + SCSI_PROT_IP_CHECKSUM,
>
> Sagi> A bit strange to me that you put REF_CHECK & REF_INCREMENT flag
> Sagi> depending on the prot_op while it really depends on the prot_type.
>
> It doesn't just depend on the prot_type. You need to be able to tell the
> HBA to increment/test the ref tag even for a Type 0 device.
>

That's still a dependence on prot_type (type 0...). Notice that you
set SCSI_PROT_REF_INCREMENT in every op index (except SCSI_PROT_NORMAL)
so my point is that it's strange to see this association.

P.S.
Now drivers can stop understanding prot_type to set DIF operations...
so once drivers stop referencing it we can remove it from scsi_cmnd.

Sagi.

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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-09-11  7:09       ` Sagi Grimberg
@ 2014-09-12  1:35         ` Martin K. Petersen
  2014-09-12 13:20           ` Sagi Grimberg
  0 siblings, 1 reply; 36+ messages in thread
From: Martin K. Petersen @ 2014-09-12  1:35 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: Martin K. Petersen, linux-scsi, axboe, hch, sagig

>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:

Sagi,

Sagi> That's still a dependence on prot_type (type 0...). Notice that
Sagi> you set SCSI_PROT_REF_INCREMENT in every op index (except
Sagi> SCSI_PROT_NORMAL) so my point is that it's strange to see this
Sagi> association.

I still don't understand your point. The mask table indicates which
flags are valid for a given protection operation. They are explicitly
DIX flags and have nothing to do with the target protection type.

Sagi> P.S.  Now drivers can stop understanding prot_type to set DIF
Sagi> operations...  so once drivers stop referencing it we can remove
Sagi> it from scsi_cmnd.

Yes. Second part of this installment removes this and a few other things
and converts qla2xxx, lpfc, mptNsas, etc. to the new interface.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-09-12  1:35         ` Martin K. Petersen
@ 2014-09-12 13:20           ` Sagi Grimberg
  2014-09-12 18:09             ` Martin K. Petersen
  0 siblings, 1 reply; 36+ messages in thread
From: Sagi Grimberg @ 2014-09-12 13:20 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi, axboe, hch, sagig

On 9/12/2014 4:35 AM, Martin K. Petersen wrote:
>>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:
>
> Sagi,
>
> Sagi> That's still a dependence on prot_type (type 0...). Notice that
> Sagi> you set SCSI_PROT_REF_INCREMENT in every op index (except
> Sagi> SCSI_PROT_NORMAL) so my point is that it's strange to see this
> Sagi> association.
>
> I still don't understand your point. The mask table indicates which
> flags are valid for a given protection operation. They are explicitly
> DIX flags and have nothing to do with the target protection type.
>

I see,
It's just confusing to see flags such as REF_INCREMENT/GUARD_CHECK/REF_CHECK
associated with a set of prot operations, but I guess it's just me.
Do you really need the mask anyway? seems like just an extra precaution
against wrong flagging.

It's just nit-picking - I do not see anything wrong with the patchset.
We can drop this if you want...

> Sagi> P.S.  Now drivers can stop understanding prot_type to set DIF
> Sagi> operations...  so once drivers stop referencing it we can remove
> Sagi> it from scsi_cmnd.
>
> Yes. Second part of this installment removes this and a few other things
> and converts qla2xxx, lpfc, mptNsas, etc. to the new interface.
>

Nice, I posted "RDMA signature feature update" preparing iSER DIF code 
to complement this change - all that is left now is a straight-forward
conversion.

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

* Re: [PATCH 14/14] sd: Honor block layer integrity handling flags
  2014-09-12 13:20           ` Sagi Grimberg
@ 2014-09-12 18:09             ` Martin K. Petersen
  0 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-09-12 18:09 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: Martin K. Petersen, linux-scsi, axboe, hch, sagig

>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:

Sagi,

Sagi> I see, It's just confusing to see flags such as
Sagi> REF_INCREMENT/GUARD_CHECK/REF_CHECK associated with a set of prot
Sagi> operations, but I guess it's just me.  Do you really need the mask
Sagi> anyway? seems like just an extra precaution against wrong
Sagi> flagging.

I did it this way to avoid having several additional special cases in
the hot path. Instead I apply a mask at the end to filter out any flags
that would be invalid for that particular protection operation. It
simplified the protection prep code significantly.

I was also afraid of drivers blindly using the flags to fill out IOCB
fields. I was guilty of that myself in the ones I converted and it
tripped firmware errors in several cases during qualification runs. So I
decided to be on the defensive side about the flags I send down to the
LLD. I'd also rather have that filtering encapsulated in single location
instead of every driver having to figure out whether it's being
presented with a valid combination of flags or not.

Sagi> Nice, I posted "RDMA signature feature update" preparing iSER DIF
Sagi> code to complement this change - all that is left now is a
Sagi> straight-forward conversion.

Great!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Block/SCSI data integrity update v3
  2014-09-11  0:23       ` Martin K. Petersen
@ 2014-09-12 18:16         ` Jens Axboe
  0 siblings, 0 replies; 36+ messages in thread
From: Jens Axboe @ 2014-09-12 18:16 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: Christoph Hellwig, linux-scsi, hch, sagig

On 09/10/2014 06:23 PM, Martin K. Petersen wrote:
>>>>>> "Jens" == Jens Axboe <axboe@fb.com> writes:
> 
> Jens> Thanks, I'll do that once Martin respins the previous series (was
> Jens> pulled due to multiple issues).
> 
> I bumped bi_rw to u64 as discussed. Will get that into -next to see if
> there's any fallout.

Don't push it into -next, start with having it in a branch somewhere
where Wu's automagic robots will pick it up. If they don't already,
email Wu and get that done!


-- 
Jens Axboe


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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-08-06 13:32   ` Sagi Grimberg
@ 2014-08-06 13:43     ` Sagi Grimberg
  0 siblings, 0 replies; 36+ messages in thread
From: Sagi Grimberg @ 2014-08-06 13:43 UTC (permalink / raw)
  To: Martin K. Petersen, linux-scsi

On 8/6/2014 4:32 PM, Sagi Grimberg wrote:
> On 7/25/2014 11:34 PM, Martin K. Petersen wrote:
>> The protection interval is not necessarily tied to the logical block
>> size of a block device. Stop using the terms "sector" and "sectors".
>>
>> Going forward we will use the term "seed" to describe the initial
>> reference tag value for a given I/O. "Interval" will be used to describe
>> the portion of the data buffer that a given piece of protection
>> information is associated with.
>>
>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> Looks good to me,
>
> Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
>

Following this patch we should modify scsi layer to compute with
interval instead of sector_size.

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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-07-25 20:34 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
  2014-07-26 15:25   ` Christoph Hellwig
@ 2014-08-06 13:32   ` Sagi Grimberg
  2014-08-06 13:43     ` Sagi Grimberg
  1 sibling, 1 reply; 36+ messages in thread
From: Sagi Grimberg @ 2014-08-06 13:32 UTC (permalink / raw)
  To: Martin K. Petersen, linux-scsi

On 7/25/2014 11:34 PM, Martin K. Petersen wrote:
> The protection interval is not necessarily tied to the logical block
> size of a block device. Stop using the terms "sector" and "sectors".
>
> Going forward we will use the term "seed" to describe the initial
> reference tag value for a given I/O. "Interval" will be used to describe
> the portion of the data buffer that a given piece of protection
> information is associated with.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Looks good to me,

Reviewed-by: Sagi Grimberg <sagig@mellanox.com>


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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-07-25 20:34 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
@ 2014-07-26 15:25   ` Christoph Hellwig
  2014-08-06 13:32   ` Sagi Grimberg
  1 sibling, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2014-07-26 15:25 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

Looks good,

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

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

* [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-07-25 20:34 Block/SCSI data integrity update v2 Martin K. Petersen
@ 2014-07-25 20:34 ` Martin K. Petersen
  2014-07-26 15:25   ` Christoph Hellwig
  2014-08-06 13:32   ` Sagi Grimberg
  0 siblings, 2 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-07-25 20:34 UTC (permalink / raw)
  To: linux-scsi; +Cc: Martin K. Petersen

The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms "sector" and "sectors".

Going forward we will use the term "seed" to describe the initial
reference tag value for a given I/O. "Interval" will be used to describe
the portion of the data buffer that a given piece of protection
information is associated with.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 block/bio-integrity.c  | 42 +++++++++++++++++++++---------------------
 block/blk-integrity.c  | 10 +++++-----
 drivers/scsi/sd_dif.c  | 46 +++++++++++++++++++++++-----------------------
 include/linux/blkdev.h |  6 +++---
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index cea93a9553d6..8c03c78281e9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -191,20 +191,20 @@ bool bio_integrity_enabled(struct bio *bio)
 EXPORT_SYMBOL(bio_integrity_enabled);
 
 /**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
  * @bi:		blk_integrity profile for device
- * @sectors:	Number of 512 sectors to convert
+ * @sectors:	Size of the bio in 512-byte sectors
  *
  * Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device.  Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device.  Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
  */
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
-						    unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+						   unsigned int sectors)
 {
 	/* At this point there are only 512b or 4096b DIF/EPP devices */
-	if (bi->sector_size == 4096)
+	if (bi->interval == 4096)
 		return sectors >>= 3;
 
 	return sectors;
@@ -213,7 +213,7 @@ static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
 static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
 					       unsigned int sectors)
 {
-	return bio_integrity_hw_sectors(bi, sectors) * bi->tuple_size;
+	return bio_integrity_intervals(bi, sectors) * bi->tuple_size;
 }
 
 /**
@@ -227,25 +227,25 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 	struct blk_integrity_exchg bix;
 	struct bio_vec *bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	sector_t sector;
-	unsigned int sectors, ret = 0, i;
+	sector_t seed;
+	unsigned int intervals, ret = 0, i;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
 
 	if (operate)
-		sector = bio->bi_iter.bi_sector;
+		seed = bio->bi_iter.bi_sector;
 	else
-		sector = bip->bip_iter.bi_sector;
+		seed = bip->bip_iter.bi_sector;
 
 	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
-	bix.sector_size = bi->sector_size;
+	bix.interval = bi->interval;
 
 	bio_for_each_segment_all(bv, bio, i) {
 		void *kaddr = kmap_atomic(bv->bv_page);
 		bix.data_buf = kaddr + bv->bv_offset;
 		bix.data_size = bv->bv_len;
 		bix.prot_buf = prot_buf;
-		bix.sector = sector;
+		bix.seed = seed;
 
 		if (operate)
 			bi->generate_fn(&bix);
@@ -257,9 +257,9 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 			}
 		}
 
-		sectors = bv->bv_len / bi->sector_size;
-		sector += sectors;
-		prot_buf += sectors * bi->tuple_size;
+		intervals = bv->bv_len / bi->interval;
+		seed += intervals;
+		prot_buf += intervals * bi->tuple_size;
 
 		kunmap_atomic(kaddr);
 	}
@@ -300,17 +300,17 @@ int bio_integrity_prep(struct bio *bio)
 	unsigned long start, end;
 	unsigned int len, nr_pages;
 	unsigned int bytes, offset, i;
-	unsigned int sectors;
+	unsigned int intervals;
 
 	bi = bdev_get_integrity(bio->bi_bdev);
 	q = bdev_get_queue(bio->bi_bdev);
 	BUG_ON(bi == NULL);
 	BUG_ON(bio_integrity(bio));
 
-	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
+	intervals = bio_integrity_intervals(bi, bio_sectors(bio));
 
 	/* Allocate kernel buffer for protection data */
-	len = sectors * bi->tuple_size;
+	len = intervals * bi->tuple_size;
 	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7ac17160ab69..3a83a7d08177 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -154,10 +154,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
 	if (!b1 || !b2)
 		return -1;
 
-	if (b1->sector_size != b2->sector_size) {
-		printk(KERN_ERR "%s: %s/%s sector sz %u != %u\n", __func__,
-		       gd1->disk_name, gd2->disk_name,
-		       b1->sector_size, b2->sector_size);
+	if (b1->interval != b2->interval) {
+		pr_err("%s: %s/%s protection interval %u != %u\n",
+		       __func__, gd1->disk_name, gd2->disk_name,
+		       b1->interval, b2->interval);
 		return -1;
 	}
 
@@ -407,7 +407,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 		kobject_uevent(&bi->kobj, KOBJ_ADD);
 
 		bi->flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
-		bi->sector_size = queue_logical_block_size(disk->queue);
+		bi->interval = queue_logical_block_size(disk->queue);
 		disk->integrity = bi;
 	} else
 		bi = disk->integrity;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 38a7778631be..1600270a46e5 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -57,16 +57,16 @@ static void sd_dif_type1_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
-		sdt->ref_tag = cpu_to_be32(sector & 0xffffffff);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
+		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 }
 
@@ -84,35 +84,35 @@ static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff)
 			return 0;
 
-		if (be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
+		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
 			printk(KERN_ERR
 			       "%s: ref tag error on sector %lu (rcvd %u)\n",
-			       bix->disk_name, (unsigned long)sector,
+			       bix->disk_name, (unsigned long)seed,
 			       be32_to_cpu(sdt->ref_tag));
 			return -EIO;
 		}
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
@@ -155,12 +155,12 @@ static void sd_dif_type3_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 	struct sd_dif_tuple *sdt = bix->prot_buf;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
 		sdt->ref_tag = 0;
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
+		buf += bix->interval;
 	}
 }
 
@@ -178,27 +178,27 @@ static int sd_dif_type3_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
 			return 0;
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 84a0fa90cdad..d097f945cf57 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1468,9 +1468,9 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 struct blk_integrity_exchg {
 	void			*prot_buf;
 	void			*data_buf;
-	sector_t		sector;
+	sector_t		seed;
 	unsigned int		data_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	const char		*disk_name;
 };
 
@@ -1483,7 +1483,7 @@ struct blk_integrity {
 
 	unsigned short		flags;
 	unsigned short		tuple_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	unsigned short		tag_size;
 
 	const char		*name;
-- 
1.9.3


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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-07-03  9:35   ` Sagi Grimberg
@ 2014-07-03 10:19     ` Sagi Grimberg
  0 siblings, 0 replies; 36+ messages in thread
From: Sagi Grimberg @ 2014-07-03 10:19 UTC (permalink / raw)
  To: Martin K. Petersen, axboe, nab, linux-scsi

On 7/3/2014 12:35 PM, Sagi Grimberg wrote:
> On 5/29/2014 6:28 AM, Martin K. Petersen wrote:
>> The protection interval is not necessarily tied to the logical block
>> size of a block device. Stop using the terms sector and sectors.
>>
>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>> ---
>>   block/bio-integrity.c  | 46 
>> +++++++++++++++++++++-------------------------
>>   block/blk-integrity.c  | 10 +++++-----
>>   drivers/scsi/sd_dif.c  | 46 
>> +++++++++++++++++++++++-----------------------
>>   include/linux/blkdev.h |  6 +++---
>>   4 files changed, 52 insertions(+), 56 deletions(-)
>>
>> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
>> index e06b3c807eef..c52a8fd98706 100644
>> --- a/block/bio-integrity.c
>> +++ b/block/bio-integrity.c
>> @@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
>>   EXPORT_SYMBOL(bio_integrity_enabled);
>>     /**
>> - * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
>> + * bio_integrity_intervals - Return number of integrity intervals 
>> for a bio
>>    * @bi:        blk_integrity profile for device
>> - * @sectors:    Number of 512 sectors to convert
>> + * @sectors:    Size of the bio in 512-byte sectors
>>    *
>>    * Description: The block layer calculates everything in 512 byte
>> - * sectors but integrity metadata is done in terms of the hardware
>> - * sector size of the storage device.  Convert the block layer sectors
>> - * to physical sectors.
>> + * sectors but integrity metadata is done in terms of the data 
>> integrity
>> + * interval size of the storage device.  Convert the block layer 
>> sectors
>> + * to the appropriate number of integrity intervals.
>>    */
>> -static inline unsigned int bio_integrity_hw_sectors(struct 
>> blk_integrity *bi,
>> -                            unsigned int sectors)
>> +static inline unsigned int bio_integrity_intervals(struct 
>> blk_integrity *bi,
>> +                           unsigned int sectors)
>>   {
>> -    /* At this point there are only 512b or 4096b DIF/EPP devices */
>> -    if (bi->sector_size == 4096)
>> -        return sectors >>= 3;
>> -
>> -    return sectors;
>> +    return sectors >> (ilog2(bi->interval) - 9);
>>   }
>
> Now that protection information interval does not necessarily match 
> the sector_size, should this routine
> protect against bogus bi->interval (e.g. fail if bi->interval < 
> sector_size for example)? Not sure
> if this check is really needed here, but it might be useful to have 
> (although protection interval
> is still effectively sector_size).
>

For v1 scsi_transfer_length should also be modified to use protection 
interval.

Sagi.

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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-05-29  3:28 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
  2014-06-11 16:45   ` Christoph Hellwig
@ 2014-07-03  9:35   ` Sagi Grimberg
  2014-07-03 10:19     ` Sagi Grimberg
  1 sibling, 1 reply; 36+ messages in thread
From: Sagi Grimberg @ 2014-07-03  9:35 UTC (permalink / raw)
  To: Martin K. Petersen, axboe, nab, linux-scsi

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:
> The protection interval is not necessarily tied to the logical block
> size of a block device. Stop using the terms sector and sectors.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>   block/bio-integrity.c  | 46 +++++++++++++++++++++-------------------------
>   block/blk-integrity.c  | 10 +++++-----
>   drivers/scsi/sd_dif.c  | 46 +++++++++++++++++++++++-----------------------
>   include/linux/blkdev.h |  6 +++---
>   4 files changed, 52 insertions(+), 56 deletions(-)
>
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index e06b3c807eef..c52a8fd98706 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
>   EXPORT_SYMBOL(bio_integrity_enabled);
>   
>   /**
> - * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
> + * bio_integrity_intervals - Return number of integrity intervals for a bio
>    * @bi:		blk_integrity profile for device
> - * @sectors:	Number of 512 sectors to convert
> + * @sectors:	Size of the bio in 512-byte sectors
>    *
>    * Description: The block layer calculates everything in 512 byte
> - * sectors but integrity metadata is done in terms of the hardware
> - * sector size of the storage device.  Convert the block layer sectors
> - * to physical sectors.
> + * sectors but integrity metadata is done in terms of the data integrity
> + * interval size of the storage device.  Convert the block layer sectors
> + * to the appropriate number of integrity intervals.
>    */
> -static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
> -						    unsigned int sectors)
> +static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
> +						   unsigned int sectors)
>   {
> -	/* At this point there are only 512b or 4096b DIF/EPP devices */
> -	if (bi->sector_size == 4096)
> -		return sectors >>= 3;
> -
> -	return sectors;
> +	return sectors >> (ilog2(bi->interval) - 9);
>   }

Now that protection information interval does not necessarily match the 
sector_size, should this routine
protect against bogus bi->interval (e.g. fail if bi->interval < 
sector_size for example)? Not sure
if this check is really needed here, but it might be useful to have 
(although protection interval
is still effectively sector_size).

>   
>   static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
>   					       unsigned int sectors)
>   {
> -	return bio_integrity_hw_sectors(bi, sectors) * bi->tuple_size;
> +	return bio_integrity_intervals(bi, sectors) * bi->tuple_size;
>   }
>   
>   /**
> @@ -227,25 +223,25 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
>   	struct blk_integrity_exchg bix;
>   	struct bio_vec *bv;
>   	struct bio_integrity_payload *bip = bio_integrity(bio);
> -	sector_t sector;
> -	unsigned int sectors, ret = 0, i;
> +	sector_t seed;
> +	unsigned int intervals, ret = 0, i;
>   	void *prot_buf = page_address(bip->bip_vec->bv_page) +
>   		bip->bip_vec->bv_offset;
>   
>   	if (operate)
> -		sector = bio->bi_iter.bi_sector;
> +		seed = bio->bi_iter.bi_sector;
>   	else
> -		sector = bip->bip_iter.bi_sector;
> +		seed = bip->bip_iter.bi_sector;
>   
>   	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
> -	bix.sector_size = bi->sector_size;
> +	bix.interval = bi->interval;
>   
>   	bio_for_each_segment_all(bv, bio, i) {
>   		void *kaddr = kmap_atomic(bv->bv_page);
>   		bix.data_buf = kaddr + bv->bv_offset;
>   		bix.data_size = bv->bv_len;
>   		bix.prot_buf = prot_buf;
> -		bix.sector = sector;
> +		bix.seed = seed;
>   
>   		if (operate)
>   			bi->generate_fn(&bix);
> @@ -257,9 +253,9 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
>   			}
>   		}
>   
> -		sectors = bv->bv_len / bi->sector_size;
> -		sector += sectors;
> -		prot_buf += sectors * bi->tuple_size;
> +		intervals = bv->bv_len / bi->interval;
> +		seed += intervals;
> +		prot_buf += intervals * bi->tuple_size;
>   
>   		kunmap_atomic(kaddr);
>   	}
> @@ -300,17 +296,17 @@ int bio_integrity_prep(struct bio *bio)
>   	unsigned long start, end;
>   	unsigned int len, nr_pages;
>   	unsigned int bytes, offset, i;
> -	unsigned int sectors;
> +	unsigned int intervals;
>   
>   	bi = bdev_get_integrity(bio->bi_bdev);
>   	q = bdev_get_queue(bio->bi_bdev);
>   	BUG_ON(bi == NULL);
>   	BUG_ON(bio_integrity(bio));
>   
> -	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
> +	intervals = bio_integrity_intervals(bi, bio_sectors(bio));
>   
>   	/* Allocate kernel buffer for protection data */
> -	len = sectors * bi->tuple_size;
> +	len = intervals * bi->tuple_size;
>   	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
>   	if (unlikely(buf == NULL)) {
>   		printk(KERN_ERR "could not allocate integrity buffer\n");
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index 7ac17160ab69..3760d0aeed92 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -154,10 +154,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
>   	if (!b1 || !b2)
>   		return -1;
>   
> -	if (b1->sector_size != b2->sector_size) {
> -		printk(KERN_ERR "%s: %s/%s sector sz %u != %u\n", __func__,
> -		       gd1->disk_name, gd2->disk_name,
> -		       b1->sector_size, b2->sector_size);
> +	if (b1->interval != b2->interval) {
> +		printk(KERN_ERR "%s: %s/%s protection interval %u != %u\n",
> +		       __func__, gd1->disk_name, gd2->disk_name,
> +		       b1->interval, b2->interval);
>   		return -1;
>   	}
>   
> @@ -407,7 +407,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
>   		kobject_uevent(&bi->kobj, KOBJ_ADD);
>   
>   		bi->flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
> -		bi->sector_size = queue_logical_block_size(disk->queue);
> +		bi->interval = queue_logical_block_size(disk->queue);
>   		disk->integrity = bi;
>   	} else
>   		bi = disk->integrity;
> diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
> index 38a7778631be..1600270a46e5 100644
> --- a/drivers/scsi/sd_dif.c
> +++ b/drivers/scsi/sd_dif.c
> @@ -57,16 +57,16 @@ static void sd_dif_type1_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
>   {
>   	void *buf = bix->data_buf;
>   	struct sd_dif_tuple *sdt = bix->prot_buf;
> -	sector_t sector = bix->sector;
> +	sector_t seed = bix->seed;
>   	unsigned int i;
>   
> -	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
> -		sdt->guard_tag = fn(buf, bix->sector_size);
> -		sdt->ref_tag = cpu_to_be32(sector & 0xffffffff);
> +	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
> +		sdt->guard_tag = fn(buf, bix->interval);
> +		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
>   		sdt->app_tag = 0;
>   
> -		buf += bix->sector_size;
> -		sector++;
> +		buf += bix->interval;
> +		seed++;
>   	}
>   }
>   
> @@ -84,35 +84,35 @@ static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
>   {
>   	void *buf = bix->data_buf;
>   	struct sd_dif_tuple *sdt = bix->prot_buf;
> -	sector_t sector = bix->sector;
> +	sector_t seed = bix->seed;
>   	unsigned int i;
>   	__u16 csum;
>   
> -	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
> +	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
>   		/* Unwritten sectors */
>   		if (sdt->app_tag == 0xffff)
>   			return 0;
>   
> -		if (be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
> +		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
>   			printk(KERN_ERR
>   			       "%s: ref tag error on sector %lu (rcvd %u)\n",
> -			       bix->disk_name, (unsigned long)sector,
> +			       bix->disk_name, (unsigned long)seed,
>   			       be32_to_cpu(sdt->ref_tag));
>   			return -EIO;
>   		}
>   
> -		csum = fn(buf, bix->sector_size);
> +		csum = fn(buf, bix->interval);
>   
>   		if (sdt->guard_tag != csum) {
>   			printk(KERN_ERR "%s: guard tag error on sector %lu " \
>   			       "(rcvd %04x, data %04x)\n", bix->disk_name,
> -			       (unsigned long)sector,
> +			       (unsigned long)seed,
>   			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
>   			return -EIO;
>   		}
>   
> -		buf += bix->sector_size;
> -		sector++;
> +		buf += bix->interval;
> +		seed++;
>   	}
>   
>   	return 0;
> @@ -155,12 +155,12 @@ static void sd_dif_type3_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
>   	struct sd_dif_tuple *sdt = bix->prot_buf;
>   	unsigned int i;
>   
> -	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
> -		sdt->guard_tag = fn(buf, bix->sector_size);
> +	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
> +		sdt->guard_tag = fn(buf, bix->interval);
>   		sdt->ref_tag = 0;
>   		sdt->app_tag = 0;
>   
> -		buf += bix->sector_size;
> +		buf += bix->interval;
>   	}
>   }
>   
> @@ -178,27 +178,27 @@ static int sd_dif_type3_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
>   {
>   	void *buf = bix->data_buf;
>   	struct sd_dif_tuple *sdt = bix->prot_buf;
> -	sector_t sector = bix->sector;
> +	sector_t seed = bix->seed;
>   	unsigned int i;
>   	__u16 csum;
>   
> -	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
> +	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
>   		/* Unwritten sectors */
>   		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
>   			return 0;
>   
> -		csum = fn(buf, bix->sector_size);
> +		csum = fn(buf, bix->interval);
>   
>   		if (sdt->guard_tag != csum) {
>   			printk(KERN_ERR "%s: guard tag error on sector %lu " \
>   			       "(rcvd %04x, data %04x)\n", bix->disk_name,
> -			       (unsigned long)sector,
> +			       (unsigned long)seed,
>   			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
>   			return -EIO;
>   		}
>   
> -		buf += bix->sector_size;
> -		sector++;
> +		buf += bix->interval;
> +		seed++;
>   	}
>   
>   	return 0;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ce42891dc386..b4fa1de92d29 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1434,9 +1434,9 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
>   struct blk_integrity_exchg {
>   	void			*prot_buf;
>   	void			*data_buf;
> -	sector_t		sector;
> +	sector_t		seed;
>   	unsigned int		data_size;
> -	unsigned short		sector_size;
> +	unsigned short		interval;
>   	const char		*disk_name;
>   };
>   
> @@ -1449,7 +1449,7 @@ struct blk_integrity {
>   
>   	unsigned short		flags;
>   	unsigned short		tuple_size;
> -	unsigned short		sector_size;
> +	unsigned short		interval;
>   	unsigned short		tag_size;
>   
>   	const char		*name;


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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-06-11 16:45   ` Christoph Hellwig
@ 2014-06-12  0:26     ` Martin K. Petersen
  0 siblings, 0 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-06-12  0:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Martin K. Petersen, axboe, nab, sagig, linux-scsi

>>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes:

Christoph> On Wed, May 28, 2014 at 11:28:39PM -0400, Martin K. Petersen wrote:
>> The protection interval is not necessarily tied to the logical block
>> size of a block device. Stop using the terms sector and sectors.

Christoph> This does more than just renaming symbols, so it needs a
Christoph> better description or even better splitting into separate
Christoph> patches.

The only thing I see that does more than a rename is the interval
calculation tweak. I'll put that in a separate patch.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-05-29  3:28 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
@ 2014-06-11 16:45   ` Christoph Hellwig
  2014-06-12  0:26     ` Martin K. Petersen
  2014-07-03  9:35   ` Sagi Grimberg
  1 sibling, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2014-06-11 16:45 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: axboe, nab, sagig, linux-scsi

On Wed, May 28, 2014 at 11:28:39PM -0400, Martin K. Petersen wrote:
> The protection interval is not necessarily tied to the logical block
> size of a block device. Stop using the terms sector and sectors.

This does more than just renaming symbols, so it needs a better
description or even better splitting into separate patches.


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

* [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
  2014-05-29  3:28 Data integrity update Martin K. Petersen
@ 2014-05-29  3:28 ` Martin K. Petersen
  2014-06-11 16:45   ` Christoph Hellwig
  2014-07-03  9:35   ` Sagi Grimberg
  0 siblings, 2 replies; 36+ messages in thread
From: Martin K. Petersen @ 2014-05-29  3:28 UTC (permalink / raw)
  To: axboe, nab, sagig, linux-scsi; +Cc: Martin K. Petersen

The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms sector and sectors.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 block/bio-integrity.c  | 46 +++++++++++++++++++++-------------------------
 block/blk-integrity.c  | 10 +++++-----
 drivers/scsi/sd_dif.c  | 46 +++++++++++++++++++++++-----------------------
 include/linux/blkdev.h |  6 +++---
 4 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e06b3c807eef..c52a8fd98706 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
 EXPORT_SYMBOL(bio_integrity_enabled);
 
 /**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
  * @bi:		blk_integrity profile for device
- * @sectors:	Number of 512 sectors to convert
+ * @sectors:	Size of the bio in 512-byte sectors
  *
  * Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device.  Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device.  Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
  */
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
-						    unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+						   unsigned int sectors)
 {
-	/* At this point there are only 512b or 4096b DIF/EPP devices */
-	if (bi->sector_size == 4096)
-		return sectors >>= 3;
-
-	return sectors;
+	return sectors >> (ilog2(bi->interval) - 9);
 }
 
 static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
 					       unsigned int sectors)
 {
-	return bio_integrity_hw_sectors(bi, sectors) * bi->tuple_size;
+	return bio_integrity_intervals(bi, sectors) * bi->tuple_size;
 }
 
 /**
@@ -227,25 +223,25 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 	struct blk_integrity_exchg bix;
 	struct bio_vec *bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	sector_t sector;
-	unsigned int sectors, ret = 0, i;
+	sector_t seed;
+	unsigned int intervals, ret = 0, i;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
 
 	if (operate)
-		sector = bio->bi_iter.bi_sector;
+		seed = bio->bi_iter.bi_sector;
 	else
-		sector = bip->bip_iter.bi_sector;
+		seed = bip->bip_iter.bi_sector;
 
 	bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
-	bix.sector_size = bi->sector_size;
+	bix.interval = bi->interval;
 
 	bio_for_each_segment_all(bv, bio, i) {
 		void *kaddr = kmap_atomic(bv->bv_page);
 		bix.data_buf = kaddr + bv->bv_offset;
 		bix.data_size = bv->bv_len;
 		bix.prot_buf = prot_buf;
-		bix.sector = sector;
+		bix.seed = seed;
 
 		if (operate)
 			bi->generate_fn(&bix);
@@ -257,9 +253,9 @@ static int bio_integrity_generate_verify(struct bio *bio, int operate)
 			}
 		}
 
-		sectors = bv->bv_len / bi->sector_size;
-		sector += sectors;
-		prot_buf += sectors * bi->tuple_size;
+		intervals = bv->bv_len / bi->interval;
+		seed += intervals;
+		prot_buf += intervals * bi->tuple_size;
 
 		kunmap_atomic(kaddr);
 	}
@@ -300,17 +296,17 @@ int bio_integrity_prep(struct bio *bio)
 	unsigned long start, end;
 	unsigned int len, nr_pages;
 	unsigned int bytes, offset, i;
-	unsigned int sectors;
+	unsigned int intervals;
 
 	bi = bdev_get_integrity(bio->bi_bdev);
 	q = bdev_get_queue(bio->bi_bdev);
 	BUG_ON(bi == NULL);
 	BUG_ON(bio_integrity(bio));
 
-	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
+	intervals = bio_integrity_intervals(bi, bio_sectors(bio));
 
 	/* Allocate kernel buffer for protection data */
-	len = sectors * bi->tuple_size;
+	len = intervals * bi->tuple_size;
 	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7ac17160ab69..3760d0aeed92 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -154,10 +154,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
 	if (!b1 || !b2)
 		return -1;
 
-	if (b1->sector_size != b2->sector_size) {
-		printk(KERN_ERR "%s: %s/%s sector sz %u != %u\n", __func__,
-		       gd1->disk_name, gd2->disk_name,
-		       b1->sector_size, b2->sector_size);
+	if (b1->interval != b2->interval) {
+		printk(KERN_ERR "%s: %s/%s protection interval %u != %u\n",
+		       __func__, gd1->disk_name, gd2->disk_name,
+		       b1->interval, b2->interval);
 		return -1;
 	}
 
@@ -407,7 +407,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 		kobject_uevent(&bi->kobj, KOBJ_ADD);
 
 		bi->flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
-		bi->sector_size = queue_logical_block_size(disk->queue);
+		bi->interval = queue_logical_block_size(disk->queue);
 		disk->integrity = bi;
 	} else
 		bi = disk->integrity;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 38a7778631be..1600270a46e5 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -57,16 +57,16 @@ static void sd_dif_type1_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
-		sdt->ref_tag = cpu_to_be32(sector & 0xffffffff);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
+		sdt->ref_tag = cpu_to_be32(seed & 0xffffffff);
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 }
 
@@ -84,35 +84,35 @@ static int sd_dif_type1_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff)
 			return 0;
 
-		if (be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
+		if (be32_to_cpu(sdt->ref_tag) != (seed & 0xffffffff)) {
 			printk(KERN_ERR
 			       "%s: ref tag error on sector %lu (rcvd %u)\n",
-			       bix->disk_name, (unsigned long)sector,
+			       bix->disk_name, (unsigned long)seed,
 			       be32_to_cpu(sdt->ref_tag));
 			return -EIO;
 		}
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
@@ -155,12 +155,12 @@ static void sd_dif_type3_generate(struct blk_integrity_exchg *bix, csum_fn *fn)
 	struct sd_dif_tuple *sdt = bix->prot_buf;
 	unsigned int i;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
-		sdt->guard_tag = fn(buf, bix->sector_size);
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
+		sdt->guard_tag = fn(buf, bix->interval);
 		sdt->ref_tag = 0;
 		sdt->app_tag = 0;
 
-		buf += bix->sector_size;
+		buf += bix->interval;
 	}
 }
 
@@ -178,27 +178,27 @@ static int sd_dif_type3_verify(struct blk_integrity_exchg *bix, csum_fn *fn)
 {
 	void *buf = bix->data_buf;
 	struct sd_dif_tuple *sdt = bix->prot_buf;
-	sector_t sector = bix->sector;
+	sector_t seed = bix->seed;
 	unsigned int i;
 	__u16 csum;
 
-	for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
+	for (i = 0 ; i < bix->data_size ; i += bix->interval, sdt++) {
 		/* Unwritten sectors */
 		if (sdt->app_tag == 0xffff && sdt->ref_tag == 0xffffffff)
 			return 0;
 
-		csum = fn(buf, bix->sector_size);
+		csum = fn(buf, bix->interval);
 
 		if (sdt->guard_tag != csum) {
 			printk(KERN_ERR "%s: guard tag error on sector %lu " \
 			       "(rcvd %04x, data %04x)\n", bix->disk_name,
-			       (unsigned long)sector,
+			       (unsigned long)seed,
 			       be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
 			return -EIO;
 		}
 
-		buf += bix->sector_size;
-		sector++;
+		buf += bix->interval;
+		seed++;
 	}
 
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ce42891dc386..b4fa1de92d29 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1434,9 +1434,9 @@ static inline uint64_t rq_io_start_time_ns(struct request *req)
 struct blk_integrity_exchg {
 	void			*prot_buf;
 	void			*data_buf;
-	sector_t		sector;
+	sector_t		seed;
 	unsigned int		data_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	const char		*disk_name;
 };
 
@@ -1449,7 +1449,7 @@ struct blk_integrity {
 
 	unsigned short		flags;
 	unsigned short		tuple_size;
-	unsigned short		sector_size;
+	unsigned short		interval;
 	unsigned short		tag_size;
 
 	const char		*name;
-- 
1.9.0


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

end of thread, other threads:[~2014-09-12 18:17 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-28 19:31 Block/SCSI data integrity update v3 Martin K. Petersen
2014-08-28 19:31 ` [PATCH 01/14] block: Get rid of bdev_integrity_enabled() Martin K. Petersen
2014-08-28 19:31 ` [PATCH 02/14] block: Replace bi_integrity with bi_special Martin K. Petersen
2014-08-28 19:31 ` [PATCH 03/14] block: Remove integrity tagging functions Martin K. Petersen
2014-08-28 19:31 ` [PATCH 04/14] block: Remove bip_buf Martin K. Petersen
2014-08-28 19:31 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
2014-08-28 19:31 ` [PATCH 06/14] block: Make protection interval calculation generic Martin K. Petersen
2014-08-28 19:31 ` [PATCH 07/14] block: Clean up the code used to generate and verify integrity metadata Martin K. Petersen
2014-08-28 19:31 ` [PATCH 08/14] block: Add prefix to block integrity profile flags Martin K. Petersen
2014-08-28 19:31 ` [PATCH 09/14] block: Add a disk flag to block integrity profile Martin K. Petersen
2014-08-28 19:31 ` [PATCH 10/14] block: Relocate bio integrity flags Martin K. Petersen
2014-08-28 19:31 ` [PATCH 11/14] block: Integrity checksum flag Martin K. Petersen
2014-08-28 19:31 ` [PATCH 12/14] block: Don't merge requests if integrity flags differ Martin K. Petersen
2014-08-28 19:31 ` [PATCH 13/14] block: Add T10 Protection Information functions Martin K. Petersen
2014-08-28 19:31 ` [PATCH 14/14] sd: Honor block layer integrity handling flags Martin K. Petersen
2014-08-31 21:17   ` Sagi Grimberg
2014-09-11  0:07     ` Martin K. Petersen
2014-09-11  7:09       ` Sagi Grimberg
2014-09-12  1:35         ` Martin K. Petersen
2014-09-12 13:20           ` Sagi Grimberg
2014-09-12 18:09             ` Martin K. Petersen
2014-08-28 20:10 ` Block/SCSI data integrity update v3 Jens Axboe
2014-08-29  0:43   ` Christoph Hellwig
2014-09-07 16:29   ` Christoph Hellwig
2014-09-08 15:50     ` Jens Axboe
2014-09-11  0:23       ` Martin K. Petersen
2014-09-12 18:16         ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2014-07-25 20:34 Block/SCSI data integrity update v2 Martin K. Petersen
2014-07-25 20:34 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
2014-07-26 15:25   ` Christoph Hellwig
2014-08-06 13:32   ` Sagi Grimberg
2014-08-06 13:43     ` Sagi Grimberg
2014-05-29  3:28 Data integrity update Martin K. Petersen
2014-05-29  3:28 ` [PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity Martin K. Petersen
2014-06-11 16:45   ` Christoph Hellwig
2014-06-12  0:26     ` Martin K. Petersen
2014-07-03  9:35   ` Sagi Grimberg
2014-07-03 10:19     ` Sagi Grimberg

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.