From: Mike Snitzer <snitzer@redhat.com>
To: "Alasdair G. Kergon" <agk@redhat.com>
Cc: Heinz Mauelshagen <heinzm@redhat.com>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-kernel@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
device-mapper development <dm-devel@redhat.com>,
Mikulas Patocka <mpatocka@redhat.com>,
Joe Thornber <ejt@redhat.com>
Subject: [PATCH v2] dm cache: fix writes to cache device in writethrough mode
Date: Mon, 25 Mar 2013 19:59:18 -0400 [thread overview]
Message-ID: <20130325235918.GA28746@redhat.com> (raw)
In-Reply-To: <20130323225648.GA13583@redhat.com>
From: Darrick J. Wong <darrick.wong@oracle.com>
The dm-cache writethrough strategy introduced by commit e2e74d617eadc15
("dm cache: fix race in writethrough implementation") issues a bio to
the origin device, remaps and then issues the bio to the cache device.
This more conservative in-series approach was selected to favor
correctness over performance (of the previous parallel writethrough).
However, this in-series implementation that reuses the same bio to write
both the origin and cache device didn't take into account that the block
layer's req_bio_endio() modifies a completing bio's bi_sector and
bi_size. So the new writethrough strategy needs to preserve these bio
fields, and restore them before submission to the cache device,
otherwise nothing gets written to the cache (because bi_size is 0).
This patch adds a struct dm_bio_details field to struct per_bio_data,
and uses dm_bio_record() and dm_bio_restore() to ensure the bio is
restored before reissuing to the cache device. Adding such a large
structure to the per_bio_data is not ideal but we can improve this
later, for now correctness is the important thing.
This problem initially went unnoticed because the dm-cache test-suite
uses a linear DM device for the dm-cache device's origin device.
Writethrough worked as expected because DM submits a *clone* of the
original bio, so the original bio which was reused for the cache was
never touched.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Joe Thornber <ejt@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm-cache-target.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
v2: revised patch header on Darrick and Joe's behalf
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index cf24a46..873f495 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -6,6 +6,7 @@
#include "dm.h"
#include "dm-bio-prison.h"
+#include "dm-bio-record.h"
#include "dm-cache-metadata.h"
#include <linux/dm-io.h>
@@ -205,6 +206,7 @@ struct per_bio_data {
struct cache *cache;
dm_cblock_t cblock;
bio_end_io_t *saved_bi_end_io;
+ struct dm_bio_details bio_details;
};
struct dm_cache_migration {
@@ -642,6 +644,7 @@ static void writethrough_endio(struct bio *bio, int err)
return;
}
+ dm_bio_restore(&pb->bio_details, bio);
remap_to_cache(pb->cache, bio, pb->cblock);
/*
@@ -665,6 +668,7 @@ static void remap_to_origin_then_cache(struct cache *cache, struct bio *bio,
pb->cache = cache;
pb->cblock = cblock;
pb->saved_bi_end_io = bio->bi_end_io;
+ dm_bio_record(&pb->bio_details, bio);
bio->bi_end_io = writethrough_endio;
remap_to_origin_clear_discard(pb->cache, bio, oblock);
next prev parent reply other threads:[~2013-03-25 23:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-22 20:11 [PATCH] dm: dm-cache fails to write the cache device in writethrough mode Darrick J. Wong
2013-03-22 20:21 ` Ben Hutchings
2013-03-22 20:50 ` Darrick J. Wong
2013-03-22 21:35 ` Mike Snitzer
2013-03-22 22:34 ` Mike Snitzer
2013-03-22 23:16 ` [dm-devel] " Darrick J. Wong
2013-03-23 3:27 ` Mike Snitzer
2013-03-23 3:48 ` [dm-devel] " Alasdair G Kergon
2013-03-23 5:15 ` Darrick J. Wong
2013-03-23 21:08 ` Mike Snitzer
2013-03-23 22:56 ` Mike Snitzer
2013-03-25 23:59 ` Mike Snitzer [this message]
2013-03-23 21:13 ` [dm-devel] " Alasdair G Kergon
2013-03-23 3:00 ` [dm-devel] [PATCH] " Alasdair G Kergon
2013-03-25 10:28 ` Joe Thornber
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=20130325235918.GA28746@redhat.com \
--to=snitzer@redhat.com \
--cc=agk@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=dm-devel@redhat.com \
--cc=ejt@redhat.com \
--cc=heinzm@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox