Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1378763 > unrolled thread

[PATCH v1 00/27] block: cleanup direct access to .bi_vcnt & .bi_io_vec

Back to article view | Back to linux.kernel

Contents

  [PATCH v1 00/27] block: cleanup direct access to .bi_vcnt & .bi_io_vec Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200
    [PATCH v1 08/27] staging: lustre: avoid to use bio->bi_vcnt directly Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200
    [PATCH v1 06/27] block: floppy: use bio_set_vec_table() Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200
    [PATCH v1 05/27] block: pktcdvd: use bio_get_base_vec() to retrive bvec table Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200
    [PATCH v1 11/27] bcache: io.c: use bio_set_vec_table Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200
    [PATCH v1 01/27] block: bio: introduce 3 helpers for cleanup Ming Lei <tom.leiming@gmail.com> - 2016-04-14 14:20 +0200

#1378763 — [PATCH v1 00/27] block: cleanup direct access to .bi_vcnt & .bi_io_vec

Hi Guys,

It is always not a good practice to access bio->bi_vcnt and
bio->bi_io_vec from drivers directly. Also this kind of direct
access will cause trouble when converting to multipage bvecs
because currently drivers may suppose one bvec always include
one page, and use the two fields to figure out how to handle
pages.

Even the actual meaning of the two fields arn't change from
block subsystem's view, but it may change from driver or fs's
view, so this patchset takes a conservative approach to cleanup
direct access to the two fields for avoiding regressions.

The 1st patch introduces the following 3 bio helpers which can be
used inside drivers for avoiding direct access to .bi_vcnt and .bi_io_vec.

        bio_pages()
        bio_get_base_vec()
        bio_set_vec_table()

bio_pages() can be easy to convert to multipage bvecs.

For bio_get_base_vec() and bio_set_vec_table(), they are often used
during initializing a new bio or in case of single bvec bio. With the
two new helpers, it becomes quite easy to audit access to .bi_io_vec
and .bi_vcnt.

Most of the other patches use the 3 helpers to clean up most of direct
access to .bi_vcnt and .bi_io_vec from drivers, except for MD and btrfs,
which two subsystems will be handled with different way in future.

For btrfs, its direct access to .bi_vcnt & .bi_io_vec need to be cleanuped
and audited that there isn't issue once converting to multipage bvecs.

For raid(md), given its usage is quite complicated, we can just
not enable multipage bvecs for raid queue until all its usage
are cleaned up and audited. So it won't be a blocker for multipage
bvecs.

Also bio_add_page() is used in floppy, dm-crypt and fs/logfs to
avoiding direct access to .bi_vcnt & .bi_io_vec.

The patchset can be found in the following tree:

https://github.com/ming1/linux/tree/v4.6-rc-block-next-mpbvecs-cleanup.v1

V1:
	- add Reviewed-by
	- remove bio_is_full() helper because target can find it
	via the return value of bio_add_pc_page() (9/27)
	- add comment on another two uses of bio_get_base_vec() (16/27)
	- rebased on latest for-next branch of block tree

Ming Lei (27):
  block: bio: introduce 3 helpers for cleanup
  block: drbd: use bio_get_base_vec() to retrieve the 1st bvec
  block: drbd: remove impossible failure handling
  block: loop: use bio_get_base_vec() to retrive bvec table
  block: pktcdvd: use bio_get_base_vec() to retrive bvec table
  block: floppy: use bio_set_vec_table()
  block: floppy: use bio_add_page()
  staging: lustre: avoid to use bio->bi_vcnt directly
  target: avoid to access .bi_vcnt directly
  bcache: debug: avoid to access .bi_io_vec directly
  bcache: io.c: use bio_set_vec_table
  bcache: journal.c: use bio_set_vec_table()
  bcache: movinggc: use bio_set_vec_table()
  bcache: writeback: use bio_set_vec_table()
  bcache: super: use bio_set_vec_table()
  bcache: super: use bio_get_base_vec
  dm: crypt: use bio_add_page()
  dm: dm-io.c: use bio_get_base_vec()
  dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
  dm: dm-bufio.c: use bio_set_vec_table()
  fs: logfs: use bio_set_vec_table()
  fs: logfs: convert to bio_add_page() in sync_request()
  fs: logfs: use bio_add_page() in __bdev_writeseg()
  fs: logfs: use bio_add_page() in do_erase()
  fs: logfs: remove unnecesary check
  kernel/power/swap.c: use bio_get_base_vec()
  mm: page_io.c: use bio_get_base_vec()

 drivers/block/drbd/drbd_bitmap.c            |   4 +-
 drivers/block/drbd/drbd_receiver.c          |  14 +---
 drivers/block/floppy.c                      |   9 +--
 drivers/block/loop.c                        |   5 +-
 drivers/block/pktcdvd.c                     |   3 +-
 drivers/md/bcache/debug.c                   |  11 ++-
 drivers/md/bcache/io.c                      |   3 +-
 drivers/md/bcache/journal.c                 |   3 +-
 drivers/md/bcache/movinggc.c                |   6 +-
 drivers/md/bcache/super.c                   |  33 ++++++---
 drivers/md/bcache/writeback.c               |   4 +-
 drivers/md/dm-bufio.c                       |   3 +-
 drivers/md/dm-crypt.c                       |   8 +--
 drivers/md/dm-io.c                          |   7 +-
 drivers/md/dm.c                             |   3 +-
 drivers/staging/lustre/lustre/llite/lloop.c |   9 +--
 drivers/target/target_core_pscsi.c          |   8 +--
 fs/logfs/dev_bdev.c                         | 107 +++++++++++-----------------
 include/linux/bio.h                         |  21 ++++++
 kernel/power/swap.c                         |  10 ++-
 mm/page_io.c                                |  18 ++++-
 21 files changed, 155 insertions(+), 134 deletions(-)

Thanks,
Ming
-- 
1.9.1

[toc] | [next] | [standalone]

#1378766 — [PATCH v1 08/27] staging: lustre: avoid to use bio->bi_vcnt directly

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/staging/lustre/lustre/llite/lloop.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index f169c0d..c7bdc2f 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -302,19 +302,20 @@ static unsigned int loop_get_bio(struct lloop_device *lo, struct bio **req)
 	}
 
 	/* TODO: need to split the bio, too bad. */
-	LASSERT(first->bi_vcnt <= LLOOP_MAX_SEGMENTS);
+	LASSERT(bio_pages(first) <= LLOOP_MAX_SEGMENTS);
 
 	rw = first->bi_rw;
 	bio = &lo->lo_bio;
 	while (*bio && (*bio)->bi_rw == rw) {
+		unsigned curr_cnt = bio_pages(*bio);
 		CDEBUG(D_INFO, "bio sector %llu size %u count %u vcnt%u\n",
 		       (unsigned long long)(*bio)->bi_iter.bi_sector,
 		       (*bio)->bi_iter.bi_size,
-		       page_count, (*bio)->bi_vcnt);
-		if (page_count + (*bio)->bi_vcnt > LLOOP_MAX_SEGMENTS)
+		       page_count, curr_cnt);
+		if (page_count + curr_cnt > LLOOP_MAX_SEGMENTS)
 			break;
 
-		page_count += (*bio)->bi_vcnt;
+		page_count += curr_cnt;
 		count++;
 		bio = &(*bio)->bi_next;
 	}
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1378768 — [PATCH v1 06/27] block: floppy: use bio_set_vec_table()

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/block/floppy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 84708a5..b5b0e68 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3811,7 +3811,7 @@ static int __floppy_read_block_0(struct block_device *bdev, int drive)
 	cbdata.drive = drive;
 
 	bio_init(&bio);
-	bio.bi_io_vec = &bio_vec;
+	bio_set_vec_table(&bio, &bio_vec, 1);
 	bio_vec.bv_page = page;
 	bio_vec.bv_len = size;
 	bio_vec.bv_offset = 0;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1378770 — [PATCH v1 05/27] block: pktcdvd: use bio_get_base_vec() to retrive bvec table

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/block/pktcdvd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index d06c62e..8f37435 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1298,7 +1298,8 @@ try_next_bio:
 static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
 {
 	int f;
-	struct bio_vec *bvec = pkt->w_bio->bi_io_vec;
+	/* need to fix this usage after multipage bvecs */
+	struct bio_vec *bvec = bio_get_base_vec(pkt->w_bio);
 
 	bio_reset(pkt->w_bio);
 	pkt->w_bio->bi_iter.bi_sector = pkt->sector;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1378772 — [PATCH v1 11/27] bcache: io.c: use bio_set_vec_table

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/bcache/io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index 86a0bb8..1c48462 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -26,8 +26,7 @@ struct bio *bch_bbio_alloc(struct cache_set *c)
 
 	bio_init(bio);
 	bio->bi_flags		|= BIO_POOL_NONE << BIO_POOL_OFFSET;
-	bio->bi_max_vecs	 = bucket_pages(c);
-	bio->bi_io_vec		 = bio->bi_inline_vecs;
+	bio_set_vec_table(bio, bio->bi_inline_vecs, bucket_pages(c));
 
 	return bio;
 }
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1378775 — [PATCH v1 01/27] block: bio: introduce 3 helpers for cleanup

Some drivers access bio->bi_vcnt and bio->bi_io_vec directly.
Firstly it isn't a good practice. Secondly it may cause trouble
for converting to multipage bvecs because currently drivers may
suppose one bvec always include one page, and use the two fields
to figure out how to handle pages.

So this patches introduces 3 helpers for cleaning up this kind
of usage.

bio_pages() can be convertd to support multipage bvecs easily.

For bio_get_base_vec() and bio_set_vec_table(), they are often
used during initializing a new bio or in case of single bvec
bio. With the two new helpers, it becomes easy to audit access
of .bi_io_vec and .bi_vcnt and check if this kind of use is good
for supporting multipage bvecs.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 include/linux/bio.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index 6b7481f..2229006d 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -310,6 +310,27 @@ static inline void bio_clear_flag(struct bio *bio, unsigned int bit)
 	bio->bi_flags &= ~(1U << bit);
 }
 
+static inline struct bio_vec *bio_get_base_vec(struct bio *bio)
+{
+	return __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
+}
+
+/* This helper should be used for setting bvec table on a new bio */
+static inline void bio_set_vec_table(struct bio *bio, struct bio_vec *table,
+			      unsigned max_vecs)
+{
+	bio->bi_io_vec = table;
+	bio->bi_max_vecs = max_vecs;
+}
+
+/* For singlepage bvecs, one segment includes one page */
+static inline unsigned bio_pages(struct bio *bio)
+{
+	if (!bio_flagged(bio, BIO_CLONED))
+		return bio->bi_vcnt;
+	return bio_segments(bio);
+}
+
 static inline void bio_get_first_bvec(struct bio *bio, struct bio_vec *bv)
 {
 	*bv = bio_iovec(bio);
-- 
1.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web