From: Mike Snitzer <snitzer@redhat.com>
To: Kent Overstreet <kmo@daterainc.com>
Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org,
linux-raid@vger.kernel.org, dm-devel@redhat.com,
linux-fsdevel@vger.kernel.org, Alasdair Kergon <agk@redhat.com>
Subject: Re: [PATCH 16/22] dm: Refactor for new bio cloning/splitting
Date: Fri, 11 Oct 2013 17:16:42 -0400 [thread overview]
Message-ID: <20131011211641.GA19775@redhat.com> (raw)
In-Reply-To: <20131011041314.GB28572@kmo>
On Fri, Oct 11 2013 at 12:13am -0400,
Kent Overstreet <kmo@daterainc.com> wrote:
> On Sun, Oct 06, 2013 at 08:14:10PM -0400, Mike Snitzer wrote:
> >
> > Please fold this fix into your for-jens branch, thanks. (Could be that
> > by the time Jens takes your immutable biovec changes we'll need to
> > rebase but at least it won't slip through the cracks).
>
> Thanks! I knew that bio chaining patch was going to cause a few issues
> like this, but it seems to useful to pass up. Anything else come up/any
> comments?
I'm wondering if there might be a cleaner way to hide the quirky nature
of the bio chaining with saved/restored bi_end_io. Joe Thornber
implemented utility wrappers local to dm-cache, see:
http://people.redhat.com/msnitzer/patches/upstream/dm-cache-for-v3.13/dm-cache-utils-for-hooking-bios.patch
would be natural to elevate something like this to the block layer and
then bury the quirk of needing to bump bi_remaining when the bi_end_io
is restored in unhook_bio().
Aside from that idea, your immutable biovec changes look sane to me. I
like how much was able to be removed from DM core. I don't see any
remaining problems that stand out. Feel free to add the following to
your DM patches in the series:
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
However, I did collect various style nits in the DM code during my
review. I'd like to see these changes applied:
Author: Mike Snitzer <snitzer@redhat.com>
Date: Fri Sep 27 18:14:38 2013 -0400
dm: style nits and comment for immutable biovec changes
---
drivers/md/dm-cache-target.c | 16 ++++++++++------
drivers/md/dm-crypt.c | 8 ++------
drivers/md/dm-io.c | 3 +--
drivers/md/dm-snap.c | 8 ++++----
drivers/md/dm-thin.c | 3 +--
drivers/md/dm-verity.c | 3 +--
drivers/md/dm.c | 6 ++----
include/linux/dm-io.h | 2 +-
8 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index a9acec6..1ccbfff 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -571,13 +571,13 @@ static void remap_to_cache(struct cache *cache, struct bio *bio,
bio->bi_bdev = cache->cache_dev->bdev;
if (!block_size_is_power_of_two(cache))
- bio->bi_iter.bi_sector = (from_cblock(cblock) *
- cache->sectors_per_block) +
- sector_div(bi_sector, cache->sectors_per_block);
+ bio->bi_iter.bi_sector =
+ (from_cblock(cblock) * cache->sectors_per_block) +
+ sector_div(bi_sector, cache->sectors_per_block);
else
- bio->bi_iter.bi_sector = (from_cblock(cblock) <<
- cache->sectors_per_block_shift) |
- (bi_sector & (cache->sectors_per_block - 1));
+ bio->bi_iter.bi_sector =
+ (from_cblock(cblock) << cache->sectors_per_block_shift) |
+ (bi_sector & (cache->sectors_per_block - 1));
}
static void check_if_tick_bio_needed(struct cache *cache, struct bio *bio)
@@ -666,6 +666,10 @@ static void writethrough_endio(struct bio *bio, int err)
struct per_bio_data *pb = get_per_bio_data(bio, PB_DATA_SIZE_WT);
bio->bi_end_io = pb->saved_bi_end_io;
+ /*
+ * Must bump bi_remaining to allow bio to complete with
+ * restored bi_end_io.
+ */
atomic_inc(&bio->bi_remaining);
if (err) {
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index e0b902f..4db7e48 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -648,12 +648,10 @@ static void crypt_convert_init(struct crypt_config *cc,
{
ctx->bio_in = bio_in;
ctx->bio_out = bio_out;
-
if (bio_in)
ctx->iter_in = bio_in->bi_iter;
if (bio_out)
ctx->iter_out = bio_out->bi_iter;
-
ctx->cc_sector = sector + cc->iv_offset;
init_completion(&ctx->restart);
}
@@ -752,8 +750,7 @@ static int crypt_convert(struct crypt_config *cc,
atomic_set(&ctx->cc_pending, 1);
- while (ctx->iter_in.bi_size &&
- ctx->iter_out.bi_size) {
+ while (ctx->iter_in.bi_size && ctx->iter_out.bi_size) {
crypt_alloc_req(cc, ctx);
@@ -1676,8 +1673,7 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
return DM_MAPIO_REMAPPED;
}
- io = crypt_io_alloc(cc, bio,
- dm_target_offset(ti, bio->bi_iter.bi_sector));
+ io = crypt_io_alloc(cc, bio, dm_target_offset(ti, bio->bi_iter.bi_sector));
if (bio_data_dir(io->base_bio) == READ) {
if (kcryptd_io_read(io, GFP_NOWAIT))
diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c
index 9cc1f3a..b2b8a10 100644
--- a/drivers/md/dm-io.c
+++ b/drivers/md/dm-io.c
@@ -307,8 +307,7 @@ static void do_region(int rw, unsigned region, struct dm_io_region *where,
dm_sector_div_up(remaining, (PAGE_SIZE >> SECTOR_SHIFT)));
bio = bio_alloc_bioset(GFP_NOIO, num_bvecs, io->client->bios);
- bio->bi_iter.bi_sector = where->sector +
- (where->count - remaining);
+ bio->bi_iter.bi_sector = where->sector + (where->count - remaining);
bio->bi_bdev = where->bdev;
bio->bi_end_io = endio;
store_io_and_region_in_bio(bio, io, region);
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 2e55bda..b7a5053 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1563,10 +1563,10 @@ static void remap_exception(struct dm_snapshot *s, struct dm_exception *e,
{
bio->bi_bdev = s->cow->bdev;
bio->bi_iter.bi_sector = chunk_to_sector(s->store,
- dm_chunk_number(e->new_chunk) +
- (chunk - e->old_chunk)) +
- (bio->bi_iter.bi_sector &
- s->store->chunk_mask);
+ dm_chunk_number(e->new_chunk) +
+ (chunk - e->old_chunk)) +
+ (bio->bi_iter.bi_sector &
+ s->store->chunk_mask);
}
static int snapshot_map(struct dm_target *ti, struct bio *bio)
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 6742927..a654024 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -1255,8 +1255,7 @@ static void process_bio_read_only(struct thin_c *tc, struct bio *bio)
r = dm_thin_find_block(tc->td, block, 1, &lookup_result);
switch (r) {
case 0:
- if (lookup_result.shared &&
- (rw == WRITE) && bio->bi_iter.bi_size)
+ if (lookup_result.shared && (rw == WRITE) && bio->bi_iter.bi_size)
bio_io_error(bio);
else {
inc_all_io_entry(tc->pool, bio);
diff --git a/drivers/md/dm-verity.c b/drivers/md/dm-verity.c
index 64d829a..ac35e95 100644
--- a/drivers/md/dm-verity.c
+++ b/drivers/md/dm-verity.c
@@ -496,8 +496,7 @@ static int verity_map(struct dm_target *ti, struct bio *bio)
io->v = v;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
- io->block = bio->bi_iter.bi_sector >>
- (v->data_dev_block_bits - SECTOR_SHIFT);
+ io->block = bio->bi_iter.bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
io->n_blocks = bio->bi_iter.bi_size >> v->data_dev_block_bits;
bio->bi_end_io = verity_end_io;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index a396137..5846801 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -539,8 +539,7 @@ static void start_io_acct(struct dm_io *io)
atomic_inc_return(&md->pending[rw]));
if (unlikely(dm_stats_used(&md->stats)))
- dm_stats_account_io(&md->stats, bio->bi_rw,
- bio->bi_iter.bi_sector,
+ dm_stats_account_io(&md->stats, bio->bi_rw, bio->bi_iter.bi_sector,
bio_sectors(bio), false, 0, &io->stats_aux);
}
@@ -558,8 +557,7 @@ static void end_io_acct(struct dm_io *io)
part_stat_unlock();
if (unlikely(dm_stats_used(&md->stats)))
- dm_stats_account_io(&md->stats, bio->bi_rw,
- bio->bi_iter.bi_sector,
+ dm_stats_account_io(&md->stats, bio->bi_rw, bio->bi_iter.bi_sector,
bio_sectors(bio), true, duration, &io->stats_aux);
/*
diff --git a/include/linux/dm-io.h b/include/linux/dm-io.h
index 6cf1f62..a68cbe5 100644
--- a/include/linux/dm-io.h
+++ b/include/linux/dm-io.h
@@ -29,7 +29,7 @@ typedef void (*io_notify_fn)(unsigned long error, void *context);
enum dm_io_mem_type {
DM_IO_PAGE_LIST,/* Page list */
- DM_IO_BIO,
+ DM_IO_BIO, /* Bio vector */
DM_IO_VMA, /* Virtual memory area */
DM_IO_KMEM, /* Kernel memory */
};
next prev parent reply other threads:[~2013-10-11 21:16 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-07 21:54 [PATCH 0/22] Immutable biovecs, block layer changes Kent Overstreet
2013-08-07 21:54 ` [PATCH 01/22] block: Use rw_copy_check_uvector() Kent Overstreet
2013-08-07 21:54 ` [PATCH 02/22] block: Consolidate duplicated bio_trim() implementations Kent Overstreet
2013-08-07 21:54 ` [PATCH 03/22] bcache: Kill unaligned bvec hack Kent Overstreet
2013-08-07 21:54 ` [PATCH 04/22] block: Abstract out bvec iterator Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 22:24 ` Geoff Levand
2013-08-08 2:04 ` Ed Cashin
2013-08-08 2:04 ` Ed Cashin
2013-08-08 2:04 ` Ed Cashin
2013-08-09 0:09 ` Kent Overstreet
2013-08-09 0:09 ` Kent Overstreet
2013-08-09 0:59 ` Ed Cashin
2013-08-09 1:05 ` Kent Overstreet
2013-08-09 20:16 ` Ed Cashin
2013-08-13 14:03 ` Ed Cashin
2013-08-13 18:51 ` Kent Overstreet
2013-08-13 19:18 ` Ed L Cashin
2013-08-07 21:54 ` [PATCH 05/22] dm: Use bvec_iter for dm_bio_record() Kent Overstreet
2013-08-07 21:54 ` [PATCH 06/22] block: Convert bio_iovec() to bvec_iter Kent Overstreet
2013-08-07 21:54 ` [PATCH 07/22] block: Convert bio_for_each_segment() " Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 21:54 ` [Cluster-devel] " Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
[not found] ` <1375912471-5106-1-git-send-email-kmo-PEzghdH756F8UrSeD/g0lQ@public.gmane.org>
2013-08-07 21:54 ` [PATCH 08/22] block: Immutable bio vecs Kent Overstreet
2013-08-07 21:54 ` Kent Overstreet
2013-08-07 21:54 ` [PATCH 09/22] block: Convert bio_copy_data() to bvec_iter Kent Overstreet
2013-08-07 21:54 ` [PATCH 10/22] bio-integrity: Convert " Kent Overstreet
2013-08-07 21:54 ` [PATCH 11/22] block: Kill bio_segments()/bi_vcnt usage Kent Overstreet
2013-08-07 21:54 ` [PATCH 12/22] block: Convert drivers to immutable biovecs Kent Overstreet
2013-08-07 21:54 ` [PATCH 13/22] ceph: Convert " Kent Overstreet
2013-08-07 21:54 ` [PATCH 14/22] block: Kill bio_iovec_idx(), __bio_iovec() Kent Overstreet
2013-08-07 21:54 ` [PATCH 15/22] rbd: Refactor bio cloning, don't clone biovecs Kent Overstreet
2013-08-07 21:54 ` [PATCH 16/22] dm: Refactor for new bio cloning/splitting Kent Overstreet
2013-09-28 4:59 ` Mike Snitzer
2013-10-03 3:17 ` Kent Overstreet
2013-10-03 3:23 ` Mike Snitzer
2013-10-03 21:45 ` Kent Overstreet
2013-10-03 22:50 ` Mike Snitzer
2013-10-04 17:07 ` Mike Snitzer
2013-10-07 0:14 ` Mike Snitzer
2013-10-11 4:13 ` Kent Overstreet
2013-10-11 21:16 ` Mike Snitzer [this message]
2013-10-11 22:11 ` Kent Overstreet
2013-08-07 21:54 ` [PATCH 17/22] block: Remove bi_idx hacks Kent Overstreet
2013-08-07 21:54 ` [PATCH 18/22] block: Generic bio chaining Kent Overstreet
2013-08-10 7:38 ` Kent Overstreet
2013-08-10 7:38 ` Kent Overstreet
2013-08-07 21:54 ` [PATCH 19/22] block: Rename bio_split() -> bio_pair_split() Kent Overstreet
2013-08-07 21:54 ` [PATCH 20/22] block: Introduce new bio_split() Kent Overstreet
2013-08-07 21:54 ` [PATCH 21/22] block: Kill bio_pair_split() Kent Overstreet
2013-08-07 21:54 ` [PATCH 22/22] block: Don't save/copy bvec array anymore, share when cloning Kent Overstreet
2013-08-08 15:09 ` [PATCH 0/22] Immutable biovecs, block layer changes Christoph Hellwig
2013-08-08 21:15 ` Kent Overstreet
2013-09-24 11:00 ` Christoph Hellwig
2013-09-24 13:20 ` Mike Snitzer
2013-09-24 14:29 ` Christoph Hellwig
2013-09-24 19:19 ` Kent Overstreet
2013-09-27 18:38 ` Mike Snitzer
2013-09-24 19:16 ` Kent Overstreet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131011211641.GA19775@redhat.com \
--to=snitzer@redhat.com \
--cc=agk@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=kmo@daterainc.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.