Re: [PATCH 02/14] block: Replace bi_integrity with bi_special

Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx> · Wed, 06 Aug 2014 16:15:04 +0300

On 7/25/2014 11:34 PM, Martin K. Petersen wrote:
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@xxxxxxxxxx>
---
  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 6818c251e937..78d8312a6528 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -77,6 +77,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:
@@ -94,7 +95,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)
@@ -134,7 +135,7 @@ static inline unsigned int bip_integrity_vecs(struct bio_integrity_payload *bip)
  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_integrity_vecs(bip)) {
@@ -235,7 +236,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;

@@ -310,12 +311,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;
@@ -511,7 +512,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);

@@ -542,7 +543,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);

@@ -564,7 +565,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);
@@ -583,7 +584,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 f11dd9efd3d9..1c608a0cbd8e 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
   */
@@ -660,8 +669,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
+	};

So I take it that we settled on a nameless union...

Looks good to me.
Reviewed-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html







