All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Hannes Reinecke <hare@suse.de>,
	Bart Van Assche <bvanassche@acm.org>,
	Christoph Hellwig <hch@lst.de>, Jan Kara <jack@suse.cz>,
	Linux Block Layer Mailinglist <linux-block@vger.kernel.org>,
	Linux FSDEVEL Mailinglist <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 1/2] block: bio: ensure newly added bio flags don't override BVEC_POOL_IDX
Date: Thu, 21 Mar 2019 08:21:55 -0600	[thread overview]
Message-ID: <17f1ef65-5fcf-0cba-4d37-a43d5e5038b0@kernel.dk> (raw)
In-Reply-To: <78cf685f-2bb4-c494-4677-afa513b47339@fb.com>

On 3/21/19 8:15 AM, Jens Axboe wrote:
> You also haven't solved the issue of now having an extra bit, 2/2 uses
> the last bit which the other patch already took...

Here's one way - kill BIO_SEG_VALID. We should just use
->bi_phys_segments to tell if it's valid or not. This patch uses -1 to
signify it's not.

Totally untested...

diff --git a/block/bio.c b/block/bio.c
index 71a78d9fb8b7..4bc165e7ca43 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -280,6 +280,7 @@ void bio_init(struct bio *bio, struct bio_vec *table,
 	      unsigned short max_vecs)
 {
 	memset(bio, 0, sizeof(*bio));
+	bio->bi_phys_segments = -1;
 	atomic_set(&bio->__bi_remaining, 1);
 	atomic_set(&bio->__bi_cnt, 1);
 
@@ -305,6 +306,7 @@ void bio_reset(struct bio *bio)
 	bio_uninit(bio);
 
 	memset(bio, 0, BIO_RESET_BYTES);
+	bio->bi_phys_segments = -1;
 	bio->bi_flags = flags;
 	atomic_set(&bio->__bi_remaining, 1);
 }
@@ -573,7 +575,7 @@ EXPORT_SYMBOL(bio_put);
 
 int bio_phys_segments(struct request_queue *q, struct bio *bio)
 {
-	if (unlikely(!bio_flagged(bio, BIO_SEG_VALID)))
+	if (unlikely(bio->bi_phys_segments == -1))
 		blk_recount_segments(q, bio);
 
 	return bio->bi_phys_segments;
@@ -731,7 +733,7 @@ int bio_add_pc_page(struct request_queue *q, struct bio *bio, struct page
 
 	/* If we may be able to merge these biovecs, force a recount */
 	if (bio->bi_vcnt > 1 && biovec_phys_mergeable(q, bvec - 1, bvec))
-		bio_clear_flag(bio, BIO_SEG_VALID);
+		bio->bi_phys_segments = -1;
 
  done:
 	return len;
@@ -1913,10 +1915,8 @@ void bio_trim(struct bio *bio, int offset, int size)
 	if (offset == 0 && size == bio->bi_iter.bi_size)
 		return;
 
-	bio_clear_flag(bio, BIO_SEG_VALID);
-
+	bio->bi_phys_segments = -1;
 	bio_advance(bio, offset << 9);
-
 	bio->bi_iter.bi_size = size;
 
 	if (bio_integrity(bio))
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 1c9d4f0f96ea..57cea2782a76 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -343,7 +343,6 @@ void blk_queue_split(struct request_queue *q, struct bio **bio)
 	/* physical segments can be figured out during splitting */
 	res = split ? split : *bio;
 	res->bi_phys_segments = nsegs;
-	bio_set_flag(res, BIO_SEG_VALID);
 
 	if (split) {
 		/* there isn't chance to merge the splitted bio */
@@ -440,8 +439,6 @@ void blk_recount_segments(struct request_queue *q, struct bio *bio)
 	bio->bi_next = NULL;
 	bio->bi_phys_segments = __blk_recalc_rq_segments(q, bio);
 	bio->bi_next = nxt;
-
-	bio_set_flag(bio, BIO_SEG_VALID);
 }
 
 static int blk_phys_contig_segment(struct request_queue *q, struct bio *bio,
@@ -651,9 +648,9 @@ int ll_back_merge_fn(struct request_queue *q, struct request *req,
 		req_set_nomerge(q, req);
 		return 0;
 	}
-	if (!bio_flagged(req->biotail, BIO_SEG_VALID))
+	if (req->biotail->bi_phys_segments == -1)
 		blk_recount_segments(q, req->biotail);
-	if (!bio_flagged(bio, BIO_SEG_VALID))
+	if (bio->bi_phys_segments == -1)
 		blk_recount_segments(q, bio);
 
 	return ll_new_hw_segment(q, req, bio);
@@ -673,9 +670,9 @@ int ll_front_merge_fn(struct request_queue *q, struct request *req,
 		req_set_nomerge(q, req);
 		return 0;
 	}
-	if (!bio_flagged(bio, BIO_SEG_VALID))
+	if (bio->bi_phys_segments == -1)
 		blk_recount_segments(q, bio);
-	if (!bio_flagged(req->bio, BIO_SEG_VALID))
+	if (req->bio->bi_phys_segments == -1)
 		blk_recount_segments(q, req->bio);
 
 	return ll_new_hw_segment(q, req, bio);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c033bfcb209e..79eb54dcf0f9 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5247,7 +5247,7 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
 		rcu_read_unlock();
 		raid_bio->bi_next = (void*)rdev;
 		bio_set_dev(align_bi, rdev->bdev);
-		bio_clear_flag(align_bi, BIO_SEG_VALID);
+		align_bi->bi_phys_segments = -1;
 
 		if (is_badblock(rdev, align_bi->bi_iter.bi_sector,
 				bio_sectors(align_bi),
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index d66bf5f32610..472059e92071 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -215,7 +215,6 @@ struct bio {
 /*
  * bio flags
  */
-#define BIO_SEG_VALID	1	/* bi_phys_segments valid */
 #define BIO_CLONED	2	/* doesn't own data */
 #define BIO_BOUNCED	3	/* bio is a bounce bio */
 #define BIO_USER_MAPPED 4	/* contains user pages */

-- 
Jens Axboe


  reply	other threads:[~2019-03-21 14:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 12:30 [PATCH 0/2] add flag for tracking bio allocation Johannes Thumshirn
2019-03-21 12:30 ` [PATCH 1/2] block: bio: ensure newly added bio flags don't override BVEC_POOL_IDX Johannes Thumshirn
2019-03-21 13:18   ` Hannes Reinecke
2019-03-21 13:22     ` Johannes Thumshirn
2019-03-21 14:15   ` Jens Axboe
2019-03-21 14:21     ` Jens Axboe [this message]
2019-03-21 14:23       ` Johannes Thumshirn
2019-03-21 14:25         ` Jens Axboe
2019-03-21 15:54           ` Hannes Reinecke
2019-03-22 10:24           ` Johannes Thumshirn
2019-03-21 12:30 ` [PATCH 2/2] bio: introduce BIO_ALLOCED flag and check it in bio_free Johannes Thumshirn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=17f1ef65-5fcf-0cba-4d37-a43d5e5038b0@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=jthumshirn@suse.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.