* [PATCH 01/12] block: bio: pass bvec table to bio_init()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-12 17:59 ` Christoph Hellwig
2016-11-11 12:05 ` [PATCH 02/12] block: drbd: remove impossible failure handling Ming Lei
` (11 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Jens Axboe, Jiri Kosina, Kent Overstreet, Shaohua Li,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Christoph Hellwig, Sagi Grimberg, Joern Engel, Prasad Joshi,
Mike Christie, Hannes Reinecke, Rasmus Villemoes,
Johannes Thumshirn, Guoqing Jiang, Eric Wheeler, Yijing Wang,
Coly Li, Zheng Liu, Keith Busch, Adrian Hunter,
open list:BCACHE (BLOCK LAYER CACHE),
open list:SOFTWARE RAID (Multiple Disks) SUPPORT,
open list:NVM EXPRESS TARGET DRIVER, open list:LogFS
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 01/12] block: bio: pass bvec table to bio_init()
2016-11-11 12:05 ` [PATCH 01/12] block: bio: pass bvec table to bio_init() Ming Lei
@ 2016-11-12 17:59 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2016-11-12 17:59 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Jens Axboe, Jiri Kosina, Kent Overstreet,
Shaohua Li, Alasdair Kergon, Mike Snitzer,
maintainer:DEVICE-MAPPER (LVM),
Christoph Hellwig, Sagi Grimberg, Joern Engel, Prasad Joshi,
Mike Christie, Hannes Reinecke, Rasmus Villemoes,
Johannes Thumshirn, Guoqing Jiang, Eric Wheeler, Yijing Wang,
Coly Li, Zheng Liu, Keith Busch, Adrian Hunter,
open list:BCACHE (BLOCK LAYER CACHE),
open list:SOFTWARE RAID (Multiple Disks) SUPPORT,
open list:NVM EXPRESS TARGET DRIVER, open list:LogFS
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>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 02/12] block: drbd: remove impossible failure handling
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
2016-11-11 12:05 ` [PATCH 01/12] block: bio: pass bvec table to bio_init() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-11 12:05 ` [PATCH 03/12] block: floppy: use bio_add_page() Ming Lei
` (10 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Philipp Reisner, Lars Ellenberg, open list:DRBD DRIVER
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 03/12] block: floppy: use bio_add_page()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
2016-11-11 12:05 ` [PATCH 01/12] block: bio: pass bvec table to bio_init() Ming Lei
2016-11-11 12:05 ` [PATCH 02/12] block: drbd: remove impossible failure handling Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-12 18:00 ` Christoph Hellwig
2016-11-11 12:05 ` [PATCH 04/12] target: avoid to access .bi_vcnt directly Ming Lei
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Jiri Kosina, Mike Christie, Hannes Reinecke, Rasmus Villemoes
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 03/12] block: floppy: use bio_add_page()
2016-11-11 12:05 ` [PATCH 03/12] block: floppy: use bio_add_page() Ming Lei
@ 2016-11-12 18:00 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2016-11-12 18:00 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Jiri Kosina, Mike Christie, Hannes Reinecke,
Rasmus Villemoes
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/12] target: avoid to access .bi_vcnt directly
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (2 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 03/12] block: floppy: use bio_add_page() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-12 21:37 ` Sagi Grimberg
2016-11-11 12:05 ` [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly Ming Lei
` (8 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Nicholas A. Bellinger, open list:TARGET SUBSYSTEM,
open list:TARGET SUBSYSTEM
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (3 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 04/12] target: avoid to access .bi_vcnt directly Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-22 7:30 ` Christoph Hellwig
2016-11-11 12:05 ` [PATCH 06/12] dm: crypt: use bio_add_page() Ming Lei
` (7 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Kent Overstreet, Shaohua Li, Mike Christie, Guoqing Jiang,
open list:BCACHE (BLOCK LAYER CACHE),
open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly
2016-11-11 12:05 ` [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly Ming Lei
@ 2016-11-22 7:30 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2016-11-22 7:30 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Kent Overstreet, Shaohua Li, Mike Christie,
Guoqing Jiang, open list:BCACHE (BLOCK LAYER CACHE),
open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 06/12] dm: crypt: use bio_add_page()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (4 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-21 14:49 ` Mike Snitzer
2016-11-11 12:05 ` [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page Ming Lei
` (6 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 06/12] dm: crypt: use bio_add_page()
2016-11-11 12:05 ` [PATCH 06/12] dm: crypt: use bio_add_page() Ming Lei
@ 2016-11-21 14:49 ` Mike Snitzer
0 siblings, 0 replies; 27+ messages in thread
From: Mike Snitzer @ 2016-11-21 14:49 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Alasdair Kergon,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (5 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 06/12] dm: crypt: use bio_add_page() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-15 1:01 ` Ming Lei
2016-11-21 14:49 ` Mike Snitzer
2016-11-11 12:05 ` [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei
` (5 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-11 12:05 ` [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page Ming Lei
@ 2016-11-15 1:01 ` Ming Lei
2016-11-15 18:55 ` Christoph Hellwig
2016-11-21 14:49 ` Mike Snitzer
1 sibling, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-15 1:01 UTC (permalink / raw)
To: Jens Axboe, Linux Kernel Mailing List
Cc: linux-block, Linux FS Devel, Christoph Hellwig, Ming Lei,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-15 1:01 ` Ming Lei
@ 2016-11-15 18:55 ` Christoph Hellwig
2016-11-15 18:57 ` Mike Snitzer
0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2016-11-15 18:55 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block,
Linux FS Devel, Christoph Hellwig, Alasdair Kergon, Mike Snitzer,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
> 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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-15 18:55 ` Christoph Hellwig
@ 2016-11-15 18:57 ` Mike Snitzer
0 siblings, 0 replies; 27+ messages in thread
From: Mike Snitzer @ 2016-11-15 18:57 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Ming Lei, Jens Axboe, Linux Kernel Mailing List, linux-block,
Linux FS Devel, Alasdair Kergon, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-11 12:05 ` [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page Ming Lei
2016-11-15 1:01 ` Ming Lei
@ 2016-11-21 14:49 ` Mike Snitzer
2016-11-22 0:26 ` Ming Lei
1 sibling, 1 reply; 27+ messages in thread
From: Mike Snitzer @ 2016-11-21 14:49 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Alasdair Kergon,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-21 14:49 ` Mike Snitzer
@ 2016-11-22 0:26 ` Ming Lei
2016-11-22 7:28 ` Christoph Hellwig
0 siblings, 1 reply; 27+ messages in thread
From: Ming Lei @ 2016-11-22 0:26 UTC (permalink / raw)
To: Mike Snitzer
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block,
Linux FS Devel, Christoph Hellwig, Alasdair Kergon,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page
2016-11-22 0:26 ` Ming Lei
@ 2016-11-22 7:28 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2016-11-22 7:28 UTC (permalink / raw)
To: Ming Lei
Cc: Mike Snitzer, Jens Axboe, Linux Kernel Mailing List, linux-block,
Linux FS Devel, Christoph Hellwig, Alasdair Kergon,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
Jens, can you apply the non-dm patches?
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (6 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-15 1:03 ` Ming Lei
2016-11-21 14:50 ` Mike Snitzer
2016-11-11 12:05 ` [PATCH 09/12] fs: logfs: convert to bio_add_page() in sync_request() Ming Lei
` (4 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
2016-11-11 12:05 ` [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei
@ 2016-11-15 1:03 ` Ming Lei
2016-11-21 14:50 ` Mike Snitzer
1 sibling, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-15 1:03 UTC (permalink / raw)
To: Jens Axboe, Linux Kernel Mailing List
Cc: linux-block, Linux FS Devel, Christoph Hellwig, Ming Lei,
Alasdair Kergon, Mike Snitzer, maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
2016-11-11 12:05 ` [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei
2016-11-15 1:03 ` Ming Lei
@ 2016-11-21 14:50 ` Mike Snitzer
1 sibling, 0 replies; 27+ messages in thread
From: Mike Snitzer @ 2016-11-21 14:50 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-kernel, linux-block, linux-fsdevel,
Christoph Hellwig, Alasdair Kergon,
maintainer:DEVICE-MAPPER (LVM),
Shaohua Li, open list:SOFTWARE RAID (Multiple Disks) SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/12] fs: logfs: convert to bio_add_page() in sync_request()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (7 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 08/12] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-11 12:05 ` [PATCH 10/12] fs: logfs: use bio_add_page() in __bdev_writeseg() Ming Lei
` (3 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Joern Engel, Prasad Joshi, open list:LogFS
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 10/12] fs: logfs: use bio_add_page() in __bdev_writeseg()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (8 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 09/12] fs: logfs: convert to bio_add_page() in sync_request() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-11 12:05 ` [PATCH 11/12] fs: logfs: use bio_add_page() in do_erase() Ming Lei
` (2 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Joern Engel, Prasad Joshi, open list:LogFS
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 11/12] fs: logfs: use bio_add_page() in do_erase()
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (9 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 10/12] fs: logfs: use bio_add_page() in __bdev_writeseg() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-11 12:05 ` [PATCH 12/12] fs: logfs: remove unnecesary check Ming Lei
2016-11-22 15:53 ` [PATCH 00/12] block: cleanup direct access to bvec table Jens Axboe
12 siblings, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Joern Engel, Prasad Joshi, open list:LogFS
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 12/12] fs: logfs: remove unnecesary check
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (10 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 11/12] fs: logfs: use bio_add_page() in do_erase() Ming Lei
@ 2016-11-11 12:05 ` Ming Lei
2016-11-22 15:53 ` [PATCH 00/12] block: cleanup direct access to bvec table Jens Axboe
12 siblings, 0 replies; 27+ messages in thread
From: Ming Lei @ 2016-11-11 12:05 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Ming Lei,
Joern Engel, Prasad Joshi, open list:LogFS
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 00/12] block: cleanup direct access to bvec table
2016-11-11 12:05 [PATCH 00/12] block: cleanup direct access to bvec table Ming Lei
` (11 preceding siblings ...)
2016-11-11 12:05 ` [PATCH 12/12] fs: logfs: remove unnecesary check Ming Lei
@ 2016-11-22 15:53 ` Jens Axboe
12 siblings, 0 replies; 27+ messages in thread
From: Jens Axboe @ 2016-11-22 15:53 UTC (permalink / raw)
To: Ming Lei, linux-kernel
Cc: linux-block, linux-fsdevel, Christoph Hellwig, Adrian Hunter,
Coly Li, open list:DEVICE-MAPPER (LVM),
open list:DRBD DRIVER, Eric Wheeler, Guoqing Jiang,
Hannes Reinecke, Jiri Kosina, Johannes Thumshirn, Keith Busch,
Kent Overstreet, open list:BCACHE (BLOCK LAYER CACHE),
open list:NVM EXPRESS TARGET DRIVER,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT,
open list:TARGET SUBSYSTEM, open list:LogFS, Mike Christie,
Mike Snitzer, Rasmus Villemoes, open list:TARGET SUBSYSTEM,
Yijing Wang, Zheng Liu
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
^ permalink raw reply [flat|nested] 27+ messages in thread