[PATCH 00/12] block: cleanup direct access to bvec table

[PATCH 00/12] block: cleanup direct access to bvec table

[Ming Lei]

Hi,

This patchset cleans up direct access to bvec table.

The 1st patch passes bvec table to bio_init(), so that
direct access to bvec table in bio initialization is avoided.

For other patches, most of them uses bio_add_page()
to replace hardcode style of adding page to bvec,
and others avoids to access bio->bi_vcnt.

The most special one is to use bvec iterator helpers
to implement .get_page/.next_page for dm-io.c

One big motivation is to prepare for supporting multipage
bvec, but this patchset is one good cleanup too even not
for that purpose.

Thanks,
Ming

Ming Lei (12):
  block: bio: pass bvec table to bio_init()
  block: drbd: remove impossible failure handling
  block: floppy: use bio_add_page()
  target: avoid to access .bi_vcnt directly
  bcache: debug: avoid to access .bi_io_vec directly
  dm: crypt: use bio_add_page()
  dm: use bvec iterator helpers to implement .get_page and .next_page
  dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
  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

 block/bio.c                        |   8 ++-
 drivers/block/drbd/drbd_receiver.c |  14 +----
 drivers/block/floppy.c             |  10 ++--
 drivers/md/bcache/debug.c          |  11 ++--
 drivers/md/bcache/io.c             |   4 +-
 drivers/md/bcache/journal.c        |   4 +-
 drivers/md/bcache/movinggc.c       |   6 +--
 drivers/md/bcache/request.c        |   2 +-
 drivers/md/bcache/super.c          |  12 ++---
 drivers/md/bcache/writeback.c      |   5 +-
 drivers/md/dm-bufio.c              |   4 +-
 drivers/md/dm-crypt.c              |   8 +--
 drivers/md/dm-io.c                 |  34 ++++++++----
 drivers/md/dm-rq.c                 |   7 ++-
 drivers/md/dm.c                    |   2 +-
 drivers/md/multipath.c             |   2 +-
 drivers/md/raid5-cache.c           |   2 +-
 drivers/md/raid5.c                 |   9 +---
 drivers/nvme/target/io-cmd.c       |   4 +-
 drivers/target/target_core_pscsi.c |   8 +--
 fs/logfs/dev_bdev.c                | 106 +++++++++++++------------------------
 include/linux/bio.h                |   3 +-
 22 files changed, 107 insertions(+), 158 deletions(-)

-- 
2.7.4

[PATCH 01/12] block: bio: pass bvec table to bio_init()

[Ming Lei]

Some drivers often use external bvec table, so introduce
this helper for this case. It is always safe to access the
bio->bi_io_vec in this way for this case.

After converting to this usage, it will becomes a bit easier
to evaluate the remaining direct access to bio->bi_io_vec,
so it can help to prepare for the following multipage bvec
support.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 block/bio.c                   |  8 ++++++--
 drivers/block/floppy.c        |  3 +--
 drivers/md/bcache/io.c        |  4 +---
 drivers/md/bcache/journal.c   |  4 +---
 drivers/md/bcache/movinggc.c  |  6 ++----
 drivers/md/bcache/request.c   |  2 +-
 drivers/md/bcache/super.c     | 12 +++---------
 drivers/md/bcache/writeback.c |  5 ++---
 drivers/md/dm-bufio.c         |  4 +---
 drivers/md/dm.c               |  2 +-
 drivers/md/multipath.c        |  2 +-
 drivers/md/raid5-cache.c      |  2 +-
 drivers/md/raid5.c            |  9 ++-------
 drivers/nvme/target/io-cmd.c  |  4 +---
 fs/logfs/dev_bdev.c           |  4 +---
 include/linux/bio.h           |  3 ++-
 16 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 2cf6ebabc68c..de257ced69b1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -270,11 +270,15 @@ static void bio_free(struct bio *bio)
 	}
 }
 
-void bio_init(struct bio *bio)
+void bio_init(struct bio *bio, struct bio_vec *table,
+	      unsigned short max_vecs)
 {
 	memset(bio, 0, sizeof(*bio));
 	atomic_set(&bio->__bi_remaining, 1);
 	atomic_set(&bio->__bi_cnt, 1);
+
+	bio->bi_io_vec = table;
+	bio->bi_max_vecs = max_vecs;
 }
 EXPORT_SYMBOL(bio_init);
 
@@ -480,7 +484,7 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
 		return NULL;
 
 	bio = p + front_pad;
-	bio_init(bio);
+	bio_init(bio, NULL, 0);
 
 	if (nr_iovecs > inline_vecs) {
 		unsigned long idx = 0;
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index e3d8e4ced4a2..6a3ff2b2e3ae 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3806,8 +3806,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_init(&bio, &bio_vec, 1);
 	bio_vec.bv_page = page;
 	bio_vec.bv_len = size;
 	bio_vec.bv_offset = 0;
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index e97b0acf7b8d..db45a88c0ce9 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -24,9 +24,7 @@ struct bio *bch_bbio_alloc(struct cache_set *c)
 	struct bbio *b = mempool_alloc(c->bio_meta, GFP_NOIO);
 	struct bio *bio = &b->bio;
 
-	bio_init(bio);
-	bio->bi_max_vecs	 = bucket_pages(c);
-	bio->bi_io_vec		 = bio->bi_inline_vecs;
+	bio_init(bio, bio->bi_inline_vecs, bucket_pages(c));
 
 	return bio;
 }
diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 6925023e12d4..1198e53d5670 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -448,13 +448,11 @@ static void do_journal_discard(struct cache *ca)
 
 		atomic_set(&ja->discard_in_flight, DISCARD_IN_FLIGHT);
 
-		bio_init(bio);
+		bio_init(bio, bio->bi_inline_vecs, 1);
 		bio_set_op_attrs(bio, REQ_OP_DISCARD, 0);
 		bio->bi_iter.bi_sector	= bucket_to_sector(ca->set,
 						ca->sb.d[ja->discard_idx]);
 		bio->bi_bdev		= ca->bdev;
-		bio->bi_max_vecs	= 1;
-		bio->bi_io_vec		= bio->bi_inline_vecs;
 		bio->bi_iter.bi_size	= bucket_bytes(ca);
 		bio->bi_end_io		= journal_discard_endio;
 
diff --git a/drivers/md/bcache/movinggc.c b/drivers/md/bcache/movinggc.c
index 5c4bddecfaf0..13b8a907006d 100644
--- a/drivers/md/bcache/movinggc.c
+++ b/drivers/md/bcache/movinggc.c
@@ -77,15 +77,13 @@ static void moving_init(struct moving_io *io)
 {
 	struct bio *bio = &io->bio.bio;
 
-	bio_init(bio);
+	bio_init(bio, bio->bi_inline_vecs,
+		 DIV_ROUND_UP(KEY_SIZE(&io->w->key), PAGE_SECTORS));
 	bio_get(bio);
 	bio_set_prio(bio, IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0));
 
 	bio->bi_iter.bi_size	= KEY_SIZE(&io->w->key) << 9;
-	bio->bi_max_vecs	= DIV_ROUND_UP(KEY_SIZE(&io->w->key),
-					       PAGE_SECTORS);
 	bio->bi_private		= &io->cl;
-	bio->bi_io_vec		= bio->bi_inline_vecs;
 	bch_bio_map(bio, NULL);
 }
 
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 0d99b5f4b3e6..f49c5417527d 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -623,7 +623,7 @@ static void do_bio_hook(struct search *s, struct bio *orig_bio)
 {
 	struct bio *bio = &s->bio.bio;
 
-	bio_init(bio);
+	bio_init(bio, NULL, 0);
 	__bio_clone_fast(bio, orig_bio);
 	bio->bi_end_io		= request_endio;
 	bio->bi_private		= &s->cl;
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 988edf928466..2fb5bfeb43e2 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1152,9 +1152,7 @@ static void register_bdev(struct cache_sb *sb, struct page *sb_page,
 	dc->bdev = bdev;
 	dc->bdev->bd_holder = dc;
 
-	bio_init(&dc->sb_bio);
-	dc->sb_bio.bi_max_vecs	= 1;
-	dc->sb_bio.bi_io_vec	= dc->sb_bio.bi_inline_vecs;
+	bio_init(&dc->sb_bio, dc->sb_bio.bi_inline_vecs, 1);
 	dc->sb_bio.bi_io_vec[0].bv_page = sb_page;
 	get_page(sb_page);
 
@@ -1814,9 +1812,7 @@ static int cache_alloc(struct cache *ca)
 	__module_get(THIS_MODULE);
 	kobject_init(&ca->kobj, &bch_cache_ktype);
 
-	bio_init(&ca->journal.bio);
-	ca->journal.bio.bi_max_vecs = 8;
-	ca->journal.bio.bi_io_vec = ca->journal.bio.bi_inline_vecs;
+	bio_init(&ca->journal.bio, ca->journal.bio.bi_inline_vecs, 8);
 
 	free = roundup_pow_of_two(ca->sb.nbuckets) >> 10;
 
@@ -1852,9 +1848,7 @@ static int register_cache(struct cache_sb *sb, struct page *sb_page,
 	ca->bdev = bdev;
 	ca->bdev->bd_holder = ca;
 
-	bio_init(&ca->sb_bio);
-	ca->sb_bio.bi_max_vecs	= 1;
-	ca->sb_bio.bi_io_vec	= ca->sb_bio.bi_inline_vecs;
+	bio_init(&ca->sb_bio, ca->sb_bio.bi_inline_vecs, 1);
 	ca->sb_bio.bi_io_vec[0].bv_page = sb_page;
 	get_page(sb_page);
 
diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
index e51644e503a5..69e1ae59cab8 100644
--- a/drivers/md/bcache/writeback.c
+++ b/drivers/md/bcache/writeback.c
@@ -106,14 +106,13 @@ static void dirty_init(struct keybuf_key *w)
 	struct dirty_io *io = w->private;
 	struct bio *bio = &io->bio;
 
-	bio_init(bio);
+	bio_init(bio, bio->bi_inline_vecs,
+		 DIV_ROUND_UP(KEY_SIZE(&w->key), PAGE_SECTORS));
 	if (!io->dc->writeback_percent)
 		bio_set_prio(bio, IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0));
 
 	bio->bi_iter.bi_size	= KEY_SIZE(&w->key) << 9;
-	bio->bi_max_vecs	= DIV_ROUND_UP(KEY_SIZE(&w->key), PAGE_SECTORS);
 	bio->bi_private		= w;
-	bio->bi_io_vec		= bio->bi_inline_vecs;
 	bch_bio_map(bio, NULL);
 }
 
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index b3ba142e59a4..262e75365cc0 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -611,9 +611,7 @@ static void use_inline_bio(struct dm_buffer *b, int rw, sector_t block,
 	char *ptr;
 	int len;
 
-	bio_init(&b->bio);
-	b->bio.bi_io_vec = b->bio_vec;
-	b->bio.bi_max_vecs = DM_BUFIO_INLINE_VECS;
+	bio_init(&b->bio, b->bio_vec, DM_BUFIO_INLINE_VECS);
 	b->bio.bi_iter.bi_sector = block << b->c->sectors_per_block_bits;
 	b->bio.bi_bdev = b->c->bdev;
 	b->bio.bi_end_io = inline_endio;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1b8b21aba34b..ffa97b742a68 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1525,7 +1525,7 @@ static struct mapped_device *alloc_dev(int minor)
 	if (!md->bdev)
 		goto bad;
 
-	bio_init(&md->flush_bio);
+	bio_init(&md->flush_bio, NULL, 0);
 	md->flush_bio.bi_bdev = md->bdev;
 	md->flush_bio.bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
 
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 673efbd6fc47..4da06d813b8f 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -130,7 +130,7 @@ static void multipath_make_request(struct mddev *mddev, struct bio * bio)
 	}
 	multipath = conf->multipaths + mp_bh->path;
 
-	bio_init(&mp_bh->bio);
+	bio_init(&mp_bh->bio, NULL, 0);
 	__bio_clone_fast(&mp_bh->bio, bio);
 
 	mp_bh->bio.bi_iter.bi_sector += multipath->rdev->data_offset;
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 2bca090cd64e..8491edcfb5a6 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1205,7 +1205,7 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
 	INIT_LIST_HEAD(&log->io_end_ios);
 	INIT_LIST_HEAD(&log->flushing_ios);
 	INIT_LIST_HEAD(&log->finished_ios);
-	bio_init(&log->flush_bio);
+	bio_init(&log->flush_bio, NULL, 0);
 
 	log->io_kc = KMEM_CACHE(r5l_io_unit, 0);
 	if (!log->io_kc)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 70acdd379e44..5f9e28443c8a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2004,13 +2004,8 @@ static struct stripe_head *alloc_stripe(struct kmem_cache *sc, gfp_t gfp,
 		for (i = 0; i < disks; i++) {
 			struct r5dev *dev = &sh->dev[i];
 
-			bio_init(&dev->req);
-			dev->req.bi_io_vec = &dev->vec;
-			dev->req.bi_max_vecs = 1;
-
-			bio_init(&dev->rreq);
-			dev->rreq.bi_io_vec = &dev->rvec;
-			dev->rreq.bi_max_vecs = 1;
+			bio_init(&dev->req, &dev->vec, 1);
+			bio_init(&dev->rreq, &dev->rvec, 1);
 		}
 	}
 	return sh;
diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index c2784cfc5e29..c58b67e61092 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -37,9 +37,7 @@ static void nvmet_inline_bio_init(struct nvmet_req *req)
 {
 	struct bio *bio = &req->inline_bio;
 
-	bio_init(bio);
-	bio->bi_max_vecs = NVMET_MAX_INLINE_BIOVEC;
-	bio->bi_io_vec = req->inline_bvec;
+	bio_init(bio, req->inline_bvec, NVMET_MAX_INLINE_BIOVEC);
 }
 
 static void nvmet_execute_rw(struct nvmet_req *req)
diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index a8329cc47dec..dc8cafeee038 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -19,9 +19,7 @@ static int sync_request(struct page *page, struct block_device *bdev, int op)
 	struct bio bio;
 	struct bio_vec bio_vec;
 
-	bio_init(&bio);
-	bio.bi_max_vecs = 1;
-	bio.bi_io_vec = &bio_vec;
+	bio_init(&bio, &bio_vec, 1);
 	bio_vec.bv_page = page;
 	bio_vec.bv_len = PAGE_SIZE;
 	bio_vec.bv_offset = 0;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index d367cd37a7f7..70a7244f08a7 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -420,7 +420,8 @@ extern int bio_phys_segments(struct request_queue *, struct bio *);
 extern int submit_bio_wait(struct bio *bio);
 extern void bio_advance(struct bio *, unsigned);
 
-extern void bio_init(struct bio *);
+extern void bio_init(struct bio *bio, struct bio_vec *table,
+		     unsigned short max_vecs);
 extern void bio_reset(struct bio *);
 void bio_chain(struct bio *, struct bio *);
 
-- 
2.7.4

[PATCH 02/12] block: drbd: remove impossible failure handling

[Ming Lei]

For a non-cloned bio, bio_add_page() only returns failure when
the io vec table is full, but in that case, bio->bi_vcnt can't
be zero at all.

So remove the impossible failure handling.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/block/drbd/drbd_receiver.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index a89538cb3eaa..c7728dd77230 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -1648,20 +1648,8 @@ int drbd_submit_peer_request(struct drbd_device *device,
 
 	page_chain_for_each(page) {
 		unsigned len = min_t(unsigned, data_size, PAGE_SIZE);
-		if (!bio_add_page(bio, page, len, 0)) {
-			/* A single page must always be possible!
-			 * But in case it fails anyways,
-			 * we deal with it, and complain (below). */
-			if (bio->bi_vcnt == 0) {
-				drbd_err(device,
-					"bio_add_page failed for len=%u, "
-					"bi_vcnt=0 (bi_sector=%llu)\n",
-					len, (uint64_t)bio->bi_iter.bi_sector);
-				err = -ENOSPC;
-				goto fail;
-			}
+		if (!bio_add_page(bio, page, len, 0))
 			goto next_bio;
-		}
 		data_size -= len;
 		sector += len >> 9;
 		--nr_pages;
-- 
2.7.4

[PATCH 03/12] block: floppy: use bio_add_page()

[Ming Lei]

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

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 6a3ff2b2e3ae..a391a3cfb3fe 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3807,12 +3807,9 @@ static int __floppy_read_block_0(struct block_device *bdev, int drive)
 	cbdata.drive = drive;
 
 	bio_init(&bio, &bio_vec, 1);
-	bio_vec.bv_page = page;
-	bio_vec.bv_len = size;
-	bio_vec.bv_offset = 0;
-	bio.bi_vcnt = 1;
-	bio.bi_iter.bi_size = size;
 	bio.bi_bdev = bdev;
+	bio_add_page(&bio, page, size, 0);
+
 	bio.bi_iter.bi_sector = 0;
 	bio.bi_flags |= (1 << BIO_QUIET);
 	bio.bi_private = &cbdata;
-- 
2.7.4

[PATCH 04/12] target: avoid to access .bi_vcnt directly

[Ming Lei]

When the bio is full, bio_add_pc_page() will return zero,
so use this way to handle full bio.

Also replace access to .bi_vcnt for pr_debug() with bio_segments().

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/target/target_core_pscsi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 9125d9358dea..04d7aa7390d0 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -935,13 +935,9 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 
 			rc = bio_add_pc_page(pdv->pdv_sd->request_queue,
 					bio, page, bytes, off);
-			if (rc != bytes)
-				goto fail;
-
 			pr_debug("PSCSI: bio->bi_vcnt: %d nr_vecs: %d\n",
-				bio->bi_vcnt, nr_vecs);
-
-			if (bio->bi_vcnt > nr_vecs) {
+				bio_segments(bio), nr_vecs);
+			if (rc != bytes) {
 				pr_debug("PSCSI: Reached bio->bi_vcnt max:"
 					" %d i: %d bio: %p, allocating another"
 					" bio\n", bio->bi_vcnt, i, bio);
-- 
2.7.4

[PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly

[Ming Lei]

Instead we use standard iterator way to do that.

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

diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c
index 1c9130ae0073..06f55056aaae 100644
--- a/drivers/md/bcache/debug.c
+++ b/drivers/md/bcache/debug.c
@@ -107,8 +107,8 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio)
 {
 	char name[BDEVNAME_SIZE];
 	struct bio *check;
-	struct bio_vec bv;
-	struct bvec_iter iter;
+	struct bio_vec bv, cbv;
+	struct bvec_iter iter, citer = { 0 };
 
 	check = bio_clone(bio, GFP_NOIO);
 	if (!check)
@@ -120,9 +120,13 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio)
 
 	submit_bio_wait(check);
 
+	citer.bi_size = UINT_MAX;
 	bio_for_each_segment(bv, bio, iter) {
 		void *p1 = kmap_atomic(bv.bv_page);
-		void *p2 = page_address(check->bi_io_vec[iter.bi_idx].bv_page);
+		void *p2;
+
+		cbv = bio_iter_iovec(check, citer);
+		p2 = page_address(cbv.bv_page);
 
 		cache_set_err_on(memcmp(p1 + bv.bv_offset,
 					p2 + bv.bv_offset,
@@ -133,6 +137,7 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio)
 				 (uint64_t) bio->bi_iter.bi_sector);
 
 		kunmap_atomic(p1);
+		bio_advance_iter(check, &citer, bv.bv_len);
 	}
 
 	bio_free_pages(check);
-- 
2.7.4

[PATCH 06/12] dm: crypt: use bio_add_page()

[Ming Lei]

We have the standard interface to add page to bio, so don't
do that in hacking way.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/dm-crypt.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 68a9eb4f3f36..e4f86bead9d3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -994,7 +994,6 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned size)
 	gfp_t gfp_mask = GFP_NOWAIT | __GFP_HIGHMEM;
 	unsigned i, len, remaining_size;
 	struct page *page;
-	struct bio_vec *bvec;
 
 retry:
 	if (unlikely(gfp_mask & __GFP_DIRECT_RECLAIM))
@@ -1019,12 +1018,7 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned size)
 
 		len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
 
-		bvec = &clone->bi_io_vec[clone->bi_vcnt++];
-		bvec->bv_page = page;
-		bvec->bv_len = len;
-		bvec->bv_offset = 0;
-
-		clone->bi_iter.bi_size += len;
+		bio_add_page(clone, page, len, 0);
 
 		remaining_size -= len;
 	}
-- 
2.7.4

[PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Ming Lei]

Firstly we have mature bvec/bio iterator helper for iterate each
page in one bio, not necessary to reinvent a wheel to do that.

Secondly the coming multipage bvecs requires this patch.

Also add comments about the direct access to bvec table.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/dm-io.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c
index 0bf1a12e35fe..2ef573c220fc 100644
--- a/drivers/md/dm-io.c
+++ b/drivers/md/dm-io.c
@@ -162,7 +162,10 @@ struct dpages {
 			 struct page **p, unsigned long *len, unsigned *offset);
 	void (*next_page)(struct dpages *dp);
 
-	unsigned context_u;
+	union {
+		unsigned context_u;
+		struct bvec_iter context_bi;
+	};
 	void *context_ptr;
 
 	void *vma_invalidate_address;
@@ -204,25 +207,36 @@ static void list_dp_init(struct dpages *dp, struct page_list *pl, unsigned offse
 static void bio_get_page(struct dpages *dp, struct page **p,
 			 unsigned long *len, unsigned *offset)
 {
-	struct bio_vec *bvec = dp->context_ptr;
-	*p = bvec->bv_page;
-	*len = bvec->bv_len - dp->context_u;
-	*offset = bvec->bv_offset + dp->context_u;
+	struct bio_vec bv = bvec_iter_bvec((struct bio_vec *)dp->context_ptr,
+			dp->context_bi);
+
+	*p = bv.bv_page;
+	*len = bv.bv_len;
+	*offset = bv.bv_offset;
+
+	/* avoid to figure out it in bio_next_page() again */
+	dp->context_bi.bi_sector = (sector_t)bv.bv_len;
 }
 
 static void bio_next_page(struct dpages *dp)
 {
-	struct bio_vec *bvec = dp->context_ptr;
-	dp->context_ptr = bvec + 1;
-	dp->context_u = 0;
+	unsigned int len = (unsigned int)dp->context_bi.bi_sector;
+
+	bvec_iter_advance((struct bio_vec *)dp->context_ptr,
+			&dp->context_bi, len);
 }
 
 static void bio_dp_init(struct dpages *dp, struct bio *bio)
 {
 	dp->get_page = bio_get_page;
 	dp->next_page = bio_next_page;
-	dp->context_ptr = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
-	dp->context_u = bio->bi_iter.bi_bvec_done;
+
+	/*
+	 * We just use bvec iterator to retrieve pages, so it is ok to
+	 * access the bvec table directly here
+	 */
+	dp->context_ptr = bio->bi_io_vec;
+	dp->context_bi = bio->bi_iter;
 }
 
 /*
-- 
2.7.4

[PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments

[Ming Lei]

Avoid to access .bi_vcnt directly, because the bio can be
splitted from block layer, and .bi_vcnt should never have
been used here.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/dm-rq.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index b2a9e2d161e4..d9cc8324e597 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -797,8 +797,13 @@ static void dm_old_request_fn(struct request_queue *q)
 		if (req_op(rq) != REQ_OP_FLUSH)
 			pos = blk_rq_pos(rq);
 
+		/*
+		 * bio can be splitted from block layer, so use
+		 * !bio_multiple_segments instead of 'bio->bi_vcnt == 1'
+		 */
 		if ((dm_old_request_peeked_before_merge_deadline(md) &&
-		     md_in_flight(md) && rq->bio && rq->bio->bi_vcnt == 1 &&
+		     md_in_flight(md) && rq->bio &&
+		     !bio_multiple_segments(rq->bio) &&
 		     md->last_rq_pos == pos && md->last_rq_rw == rq_data_dir(rq)) ||
 		    (ti->type->busy && ti->type->busy(ti))) {
 			blk_delay_queue(q, 10);
-- 
2.7.4

[PATCH 09/12] fs: logfs: convert to bio_add_page() in sync_request()

[Ming Lei]

Always bio_add_page() is the standard and preferred way to
do the task.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 fs/logfs/dev_bdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index dc8cafeee038..e01075790600 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -20,13 +20,9 @@ static int sync_request(struct page *page, struct block_device *bdev, int op)
 	struct bio_vec bio_vec;
 
 	bio_init(&bio, &bio_vec, 1);
-	bio_vec.bv_page = page;
-	bio_vec.bv_len = PAGE_SIZE;
-	bio_vec.bv_offset = 0;
-	bio.bi_vcnt = 1;
 	bio.bi_bdev = bdev;
+	bio_add_page(&bio, page, PAGE_SIZE, 0);
 	bio.bi_iter.bi_sector = page->index * (PAGE_SIZE >> 9);
-	bio.bi_iter.bi_size = PAGE_SIZE;
 	bio_set_op_attrs(&bio, op, 0);
 
 	return submit_bio_wait(&bio);
-- 
2.7.4

[PATCH 10/12] fs: logfs: use bio_add_page() in __bdev_writeseg()

[Ming Lei]

Also this patch simplify the code a bit.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 fs/logfs/dev_bdev.c | 51 ++++++++++++++++++++-------------------------------
 1 file changed, 20 insertions(+), 31 deletions(-)

diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index e01075790600..a2e8511b89d6 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -71,56 +71,45 @@ static int __bdev_writeseg(struct super_block *sb, u64 ofs, pgoff_t index,
 {
 	struct logfs_super *super = logfs_super(sb);
 	struct address_space *mapping = super->s_mapping_inode->i_mapping;
-	struct bio *bio;
+	struct bio *bio = NULL;
 	struct page *page;
 	unsigned int max_pages;
-	int i;
+	int i, ret;
 
 	max_pages = min_t(size_t, nr_pages, BIO_MAX_PAGES);
 
-	bio = bio_alloc(GFP_NOFS, max_pages);
-	BUG_ON(!bio);
-
 	for (i = 0; i < nr_pages; i++) {
-		if (i >= max_pages) {
-			/* Block layer cannot split bios :( */
-			bio->bi_vcnt = i;
-			bio->bi_iter.bi_size = i * PAGE_SIZE;
+		if (!bio) {
+			bio = bio_alloc(GFP_NOFS, max_pages);
+			BUG_ON(!bio);
+
 			bio->bi_bdev = super->s_bdev;
 			bio->bi_iter.bi_sector = ofs >> 9;
 			bio->bi_private = sb;
 			bio->bi_end_io = writeseg_end_io;
 			bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-			atomic_inc(&super->s_pending_writes);
-			submit_bio(bio);
-
-			ofs += i * PAGE_SIZE;
-			index += i;
-			nr_pages -= i;
-			i = 0;
-
-			bio = bio_alloc(GFP_NOFS, max_pages);
-			BUG_ON(!bio);
 		}
 		page = find_lock_page(mapping, index + i);
 		BUG_ON(!page);
-		bio->bi_io_vec[i].bv_page = page;
-		bio->bi_io_vec[i].bv_len = PAGE_SIZE;
-		bio->bi_io_vec[i].bv_offset = 0;
+		ret = bio_add_page(bio, page, PAGE_SIZE, 0);
 
 		BUG_ON(PageWriteback(page));
 		set_page_writeback(page);
 		unlock_page(page);
+
+		if (!ret) {
+			/* Block layer cannot split bios :( */
+			ofs += bio->bi_iter.bi_size;
+			atomic_inc(&super->s_pending_writes);
+			submit_bio(bio);
+			bio = NULL;
+		}
+	}
+
+	if (bio) {
+		atomic_inc(&super->s_pending_writes);
+		submit_bio(bio);
 	}
-	bio->bi_vcnt = nr_pages;
-	bio->bi_iter.bi_size = nr_pages * PAGE_SIZE;
-	bio->bi_bdev = super->s_bdev;
-	bio->bi_iter.bi_sector = ofs >> 9;
-	bio->bi_private = sb;
-	bio->bi_end_io = writeseg_end_io;
-	bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-	atomic_inc(&super->s_pending_writes);
-	submit_bio(bio);
 	return 0;
 }
 
-- 
2.7.4

[PATCH 11/12] fs: logfs: use bio_add_page() in do_erase()

[Ming Lei]

Also code gets simplified a bit.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 fs/logfs/dev_bdev.c | 44 +++++++++++++++-----------------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index a2e8511b89d6..b9188be0bf83 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -153,49 +153,35 @@ static int do_erase(struct super_block *sb, u64 ofs, pgoff_t index,
 		size_t nr_pages)
 {
 	struct logfs_super *super = logfs_super(sb);
-	struct bio *bio;
+	struct bio *bio = NULL;
 	unsigned int max_pages;
-	int i;
+	int i, ret;
 
 	max_pages = min_t(size_t, nr_pages, BIO_MAX_PAGES);
 
-	bio = bio_alloc(GFP_NOFS, max_pages);
-	BUG_ON(!bio);
-
 	for (i = 0; i < nr_pages; i++) {
-		if (i >= max_pages) {
-			/* Block layer cannot split bios :( */
-			bio->bi_vcnt = i;
-			bio->bi_iter.bi_size = i * PAGE_SIZE;
+		if (!bio) {
+			bio = bio_alloc(GFP_NOFS, max_pages);
+			BUG_ON(!bio);
+
 			bio->bi_bdev = super->s_bdev;
 			bio->bi_iter.bi_sector = ofs >> 9;
 			bio->bi_private = sb;
 			bio->bi_end_io = erase_end_io;
 			bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
+		}
+		ret = bio_add_page(bio, super->s_erase_page, PAGE_SIZE, 0);
+		if (!ret) {
+			/* Block layer cannot split bios :( */
+			ofs += bio->bi_iter.bi_size;
 			atomic_inc(&super->s_pending_writes);
 			submit_bio(bio);
-
-			ofs += i * PAGE_SIZE;
-			index += i;
-			nr_pages -= i;
-			i = 0;
-
-			bio = bio_alloc(GFP_NOFS, max_pages);
-			BUG_ON(!bio);
 		}
-		bio->bi_io_vec[i].bv_page = super->s_erase_page;
-		bio->bi_io_vec[i].bv_len = PAGE_SIZE;
-		bio->bi_io_vec[i].bv_offset = 0;
 	}
-	bio->bi_vcnt = nr_pages;
-	bio->bi_iter.bi_size = nr_pages * PAGE_SIZE;
-	bio->bi_bdev = super->s_bdev;
-	bio->bi_iter.bi_sector = ofs >> 9;
-	bio->bi_private = sb;
-	bio->bi_end_io = erase_end_io;
-	bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-	atomic_inc(&super->s_pending_writes);
-	submit_bio(bio);
+	if (bio) {
+		atomic_inc(&super->s_pending_writes);
+		submit_bio(bio);
+	}
 	return 0;
 }
 
-- 
2.7.4

[PATCH 12/12] fs: logfs: remove unnecesary check

[Ming Lei]

The check on bio->bi_vcnt doesn't make sense in erase_end_io().

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 fs/logfs/dev_bdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index b9188be0bf83..9bfa0151d7c9 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -143,7 +143,6 @@ static void erase_end_io(struct bio *bio)
 	struct logfs_super *super = logfs_super(sb); 
 
 	BUG_ON(bio->bi_error); /* FIXME: Retry io or write elsewhere */ 
-	BUG_ON(bio->bi_vcnt == 0); 
 	bio_put(bio); 
 	if (atomic_dec_and_test(&super->s_pending_writes))
 		wake_up(&wq); 
-- 
2.7.4

Re: [PATCH 01/12] block: bio: pass bvec table to bio_init()

[Christoph Hellwig]

On Fri, Nov 11, 2016 at 08:05:29PM +0800, Ming Lei wrote:
> Some drivers often use external bvec table, so introduce
> this helper for this case. It is always safe to access the
> bio->bi_io_vec in this way for this case.
> 
> After converting to this usage, it will becomes a bit easier
> to evaluate the remaining direct access to bio->bi_io_vec,
> so it can help to prepare for the following multipage bvec
> support.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  block/bio.c                   |  8 ++++++--
>  drivers/block/floppy.c        |  3 +--
>  drivers/md/bcache/io.c        |  4 +---
>  drivers/md/bcache/journal.c   |  4 +---
>  drivers/md/bcache/movinggc.c  |  6 ++----
>  drivers/md/bcache/request.c   |  2 +-
>  drivers/md/bcache/super.c     | 12 +++---------
>  drivers/md/bcache/writeback.c |  5 ++---
>  drivers/md/dm-bufio.c         |  4 +---
>  drivers/md/dm.c               |  2 +-
>  drivers/md/multipath.c        |  2 +-
>  drivers/md/raid5-cache.c      |  2 +-
>  drivers/md/raid5.c            |  9 ++-------
>  drivers/nvme/target/io-cmd.c  |  4 +---
>  fs/logfs/dev_bdev.c           |  4 +---
>  include/linux/bio.h           |  3 ++-
>  16 files changed, 27 insertions(+), 47 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 2cf6ebabc68c..de257ced69b1 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -270,11 +270,15 @@ static void bio_free(struct bio *bio)
>  	}
>  }
>  
> -void bio_init(struct bio *bio)
> +void bio_init(struct bio *bio, struct bio_vec *table,
> +	      unsigned short max_vecs)
>  {
>  	memset(bio, 0, sizeof(*bio));
>  	atomic_set(&bio->__bi_remaining, 1);
>  	atomic_set(&bio->__bi_cnt, 1);
> +
> +	bio->bi_io_vec = table;
> +	bio->bi_max_vecs = max_vecs;
>  }
>  EXPORT_SYMBOL(bio_init);
>  
> @@ -480,7 +484,7 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
>  		return NULL;
>  
>  	bio = p + front_pad;
> -	bio_init(bio);
> +	bio_init(bio, NULL, 0);
>  
>  	if (nr_iovecs > inline_vecs) {
>  		unsigned long idx = 0;
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index e3d8e4ced4a2..6a3ff2b2e3ae 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -3806,8 +3806,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_init(&bio, &bio_vec, 1);
>  	bio_vec.bv_page = page;
>  	bio_vec.bv_len = size;
>  	bio_vec.bv_offset = 0;
> diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
> index e97b0acf7b8d..db45a88c0ce9 100644
> --- a/drivers/md/bcache/io.c
> +++ b/drivers/md/bcache/io.c
> @@ -24,9 +24,7 @@ struct bio *bch_bbio_alloc(struct cache_set *c)
>  	struct bbio *b = mempool_alloc(c->bio_meta, GFP_NOIO);
>  	struct bio *bio = &b->bio;
>  
> -	bio_init(bio);
> -	bio->bi_max_vecs	 = bucket_pages(c);
> -	bio->bi_io_vec		 = bio->bi_inline_vecs;
> +	bio_init(bio, bio->bi_inline_vecs, bucket_pages(c));
>  
>  	return bio;
>  }
> diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
> index 6925023e12d4..1198e53d5670 100644
> --- a/drivers/md/bcache/journal.c
> +++ b/drivers/md/bcache/journal.c
> @@ -448,13 +448,11 @@ static void do_journal_discard(struct cache *ca)
>  
>  		atomic_set(&ja->discard_in_flight, DISCARD_IN_FLIGHT);
>  
> -		bio_init(bio);
> +		bio_init(bio, bio->bi_inline_vecs, 1);
>  		bio_set_op_attrs(bio, REQ_OP_DISCARD, 0);
>  		bio->bi_iter.bi_sector	= bucket_to_sector(ca->set,
>  						ca->sb.d[ja->discard_idx]);
>  		bio->bi_bdev		= ca->bdev;
> -		bio->bi_max_vecs	= 1;
> -		bio->bi_io_vec		= bio->bi_inline_vecs;
>  		bio->bi_iter.bi_size	= bucket_bytes(ca);
>  		bio->bi_end_io		= journal_discard_endio;
>  
> diff --git a/drivers/md/bcache/movinggc.c b/drivers/md/bcache/movinggc.c
> index 5c4bddecfaf0..13b8a907006d 100644
> --- a/drivers/md/bcache/movinggc.c
> +++ b/drivers/md/bcache/movinggc.c
> @@ -77,15 +77,13 @@ static void moving_init(struct moving_io *io)
>  {
>  	struct bio *bio = &io->bio.bio;
>  
> -	bio_init(bio);
> +	bio_init(bio, bio->bi_inline_vecs,
> +		 DIV_ROUND_UP(KEY_SIZE(&io->w->key), PAGE_SECTORS));
>  	bio_get(bio);
>  	bio_set_prio(bio, IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0));
>  
>  	bio->bi_iter.bi_size	= KEY_SIZE(&io->w->key) << 9;
> -	bio->bi_max_vecs	= DIV_ROUND_UP(KEY_SIZE(&io->w->key),
> -					       PAGE_SECTORS);
>  	bio->bi_private		= &io->cl;
> -	bio->bi_io_vec		= bio->bi_inline_vecs;
>  	bch_bio_map(bio, NULL);
>  }
>  
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 0d99b5f4b3e6..f49c5417527d 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -623,7 +623,7 @@ static void do_bio_hook(struct search *s, struct bio *orig_bio)
>  {
>  	struct bio *bio = &s->bio.bio;
>  
> -	bio_init(bio);
> +	bio_init(bio, NULL, 0);
>  	__bio_clone_fast(bio, orig_bio);

We have this pattern multiple times, and it almost screams for a helper.
But I think we're better off letting your patch go in as-is and sort
that out later instead of delaying it.

Otherwise this looks fine:

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

Re: [PATCH 03/12] block: floppy: use bio_add_page()

[Christoph Hellwig]

Looks fine,

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

Re: [PATCH 04/12] target: avoid to access .bi_vcnt directly

[Sagi Grimberg]

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

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Ming Lei]

On Fri, Nov 11, 2016 at 8:05 PM, Ming Lei <tom.leiming@gmail.com> wrote:
> Firstly we have mature bvec/bio iterator helper for iterate each
> page in one bio, not necessary to reinvent a wheel to do that.
>
> Secondly the coming multipage bvecs requires this patch.
>
> Also add comments about the direct access to bvec table.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  drivers/md/dm-io.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c
> index 0bf1a12e35fe..2ef573c220fc 100644
> --- a/drivers/md/dm-io.c
> +++ b/drivers/md/dm-io.c
> @@ -162,7 +162,10 @@ struct dpages {
>                          struct page **p, unsigned long *len, unsigned *offset);
>         void (*next_page)(struct dpages *dp);
>
> -       unsigned context_u;
> +       union {
> +               unsigned context_u;
> +               struct bvec_iter context_bi;
> +       };
>         void *context_ptr;
>
>         void *vma_invalidate_address;
> @@ -204,25 +207,36 @@ static void list_dp_init(struct dpages *dp, struct page_list *pl, unsigned offse
>  static void bio_get_page(struct dpages *dp, struct page **p,
>                          unsigned long *len, unsigned *offset)
>  {
> -       struct bio_vec *bvec = dp->context_ptr;
> -       *p = bvec->bv_page;
> -       *len = bvec->bv_len - dp->context_u;
> -       *offset = bvec->bv_offset + dp->context_u;
> +       struct bio_vec bv = bvec_iter_bvec((struct bio_vec *)dp->context_ptr,
> +                       dp->context_bi);
> +
> +       *p = bv.bv_page;
> +       *len = bv.bv_len;
> +       *offset = bv.bv_offset;
> +
> +       /* avoid to figure out it in bio_next_page() again */
> +       dp->context_bi.bi_sector = (sector_t)bv.bv_len;
>  }
>
>  static void bio_next_page(struct dpages *dp)
>  {
> -       struct bio_vec *bvec = dp->context_ptr;
> -       dp->context_ptr = bvec + 1;
> -       dp->context_u = 0;
> +       unsigned int len = (unsigned int)dp->context_bi.bi_sector;
> +
> +       bvec_iter_advance((struct bio_vec *)dp->context_ptr,
> +                       &dp->context_bi, len);
>  }
>
>  static void bio_dp_init(struct dpages *dp, struct bio *bio)
>  {
>         dp->get_page = bio_get_page;
>         dp->next_page = bio_next_page;
> -       dp->context_ptr = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
> -       dp->context_u = bio->bi_iter.bi_bvec_done;
> +
> +       /*
> +        * We just use bvec iterator to retrieve pages, so it is ok to
> +        * access the bvec table directly here
> +        */
> +       dp->context_ptr = bio->bi_io_vec;
> +       dp->context_bi = bio->bi_iter;
>  }

Hi Alasdair, Mike, Christoph and anyone,

Could you give this one a review?

>From my test, it just works fine, and I believe it is a good cleanup.

Thanks,
Ming Lei

Re: [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments

[Ming Lei]

On Fri, Nov 11, 2016 at 8:05 PM, Ming Lei <tom.leiming@gmail.com> wrote:
> Avoid to access .bi_vcnt directly, because the bio can be
> splitted from block layer, and .bi_vcnt should never have
> been used here.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  drivers/md/dm-rq.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index b2a9e2d161e4..d9cc8324e597 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -797,8 +797,13 @@ static void dm_old_request_fn(struct request_queue *q)
>                 if (req_op(rq) != REQ_OP_FLUSH)
>                         pos = blk_rq_pos(rq);
>
> +               /*
> +                * bio can be splitted from block layer, so use
> +                * !bio_multiple_segments instead of 'bio->bi_vcnt == 1'
> +                */
>                 if ((dm_old_request_peeked_before_merge_deadline(md) &&
> -                    md_in_flight(md) && rq->bio && rq->bio->bi_vcnt == 1 &&
> +                    md_in_flight(md) && rq->bio &&
> +                    !bio_multiple_segments(rq->bio) &&
>                      md->last_rq_pos == pos && md->last_rq_rw == rq_data_dir(rq)) ||
>                     (ti->type->busy && ti->type->busy(ti))) {
>                         blk_delay_queue(q, 10);

Hi Alasdair, Mike, Christoph and anyone,

Could you give this one a review?

Thanks,
Ming Lei

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Christoph Hellwig]

> Hi Alasdair, Mike, Christoph and anyone,
> 
> Could you give this one a review?

It looks nice, but I don't understand the code anywhere near well
enough to review it.  We'll need someone from the DM to look over it.

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Mike Snitzer]

On Tue, Nov 15 2016 at  1:55pm -0500,
Christoph Hellwig <hch@infradead.org> wrote:

> > Hi Alasdair, Mike, Christoph and anyone,
> > 
> > Could you give this one a review?
> 
> It looks nice, but I don't understand the code anywhere near well
> enough to review it.  We'll need someone from the DM to look over it.

I'll try to get to it this week.

Re: [PATCH 06/12] dm: crypt: use bio_add_page()

[Mike Snitzer]

On Fri, Nov 11 2016 at  7:05am -0500,
Ming Lei <tom.leiming@gmail.com> wrote:

> We have the standard interface to add page to bio, so don't
> do that in hacking way.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

I've staged this for 4.10

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Mike Snitzer]

On Fri, Nov 11 2016 at  7:05am -0500,
Ming Lei <tom.leiming@gmail.com> wrote:

> Firstly we have mature bvec/bio iterator helper for iterate each
> page in one bio, not necessary to reinvent a wheel to do that.
> 
> Secondly the coming multipage bvecs requires this patch.
> 
> Also add comments about the direct access to bvec table.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

I've staged this for 4.10

Re: [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments

[Mike Snitzer]

On Fri, Nov 11 2016 at  7:05am -0500,
Ming Lei <tom.leiming@gmail.com> wrote:

> Avoid to access .bi_vcnt directly, because the bio can be
> splitted from block layer, and .bi_vcnt should never have
> been used here.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

I've staged this for 4.10

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Ming Lei]

On Mon, Nov 21, 2016 at 10:49 PM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Fri, Nov 11 2016 at  7:05am -0500,
> Ming Lei <tom.leiming@gmail.com> wrote:
>
>> Firstly we have mature bvec/bio iterator helper for iterate each
>> page in one bio, not necessary to reinvent a wheel to do that.
>>
>> Secondly the coming multipage bvecs requires this patch.
>>
>> Also add comments about the direct access to bvec table.
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>
> I've staged this for 4.10

Thanks Mike!

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Christoph Hellwig]

Jens, can you apply the non-dm patches?

Re: [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly

[Christoph Hellwig]

On Fri, Nov 11, 2016 at 08:05:33PM +0800, Ming Lei wrote:
> Instead we use standard iterator way to do that.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

Looks fine,

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

Re: [PATCH 00/12] block: cleanup direct access to bvec table

[Jens Axboe]

On 11/11/2016 05:05 AM, Ming Lei wrote:
> Hi,
>
> This patchset cleans up direct access to bvec table.
>
> The 1st patch passes bvec table to bio_init(), so that
> direct access to bvec table in bio initialization is avoided.
>
> For other patches, most of them uses bio_add_page()
> to replace hardcode style of adding page to bvec,
> and others avoids to access bio->bi_vcnt.
>
> The most special one is to use bvec iterator helpers
> to implement .get_page/.next_page for dm-io.c
>
> One big motivation is to prepare for supporting multipage
> bvec, but this patchset is one good cleanup too even not
> for that purpose.

I've added the series, except 6-8, the dm parts. I updated some of the
descriptions/subjects to read a little better.

-- 
Jens Axboe