diff options
author | Darrick J. Wong <darrick.wong@oracle.com> | 2013-04-05 10:36:32 -0400 |
---|---|---|
committer | Alasdair G Kergon <agk@redhat.com> | 2013-04-05 10:36:32 -0400 |
commit | b844fe691897221ad0d5e0279c8ea9e3e4a46982 (patch) | |
tree | aff4244920ea1fcc3247d1532c72ad416fd67972 /drivers/md | |
parent | 07961ac7c0ee8b546658717034fe692fd12eefa9 (diff) |
dm cache: fix writes to cache device in writethrough mode
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>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Diffstat (limited to 'drivers/md')
-rw-r--r-- | drivers/md/dm-cache-target.c | 4 |
1 files changed, 4 insertions, 0 deletions
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c index 66120bd46d15..1ab122a75764 100644 --- a/drivers/md/dm-cache-target.c +++ b/drivers/md/dm-cache-target.c | |||
@@ -6,6 +6,7 @@ | |||
6 | 6 | ||
7 | #include "dm.h" | 7 | #include "dm.h" |
8 | #include "dm-bio-prison.h" | 8 | #include "dm-bio-prison.h" |
9 | #include "dm-bio-record.h" | ||
9 | #include "dm-cache-metadata.h" | 10 | #include "dm-cache-metadata.h" |
10 | 11 | ||
11 | #include <linux/dm-io.h> | 12 | #include <linux/dm-io.h> |
@@ -205,6 +206,7 @@ struct per_bio_data { | |||
205 | struct cache *cache; | 206 | struct cache *cache; |
206 | dm_cblock_t cblock; | 207 | dm_cblock_t cblock; |
207 | bio_end_io_t *saved_bi_end_io; | 208 | bio_end_io_t *saved_bi_end_io; |
209 | struct dm_bio_details bio_details; | ||
208 | }; | 210 | }; |
209 | 211 | ||
210 | struct dm_cache_migration { | 212 | struct dm_cache_migration { |
@@ -643,6 +645,7 @@ static void writethrough_endio(struct bio *bio, int err) | |||
643 | return; | 645 | return; |
644 | } | 646 | } |
645 | 647 | ||
648 | dm_bio_restore(&pb->bio_details, bio); | ||
646 | remap_to_cache(pb->cache, bio, pb->cblock); | 649 | remap_to_cache(pb->cache, bio, pb->cblock); |
647 | 650 | ||
648 | /* | 651 | /* |
@@ -667,6 +670,7 @@ static void remap_to_origin_then_cache(struct cache *cache, struct bio *bio, | |||
667 | pb->cache = cache; | 670 | pb->cache = cache; |
668 | pb->cblock = cblock; | 671 | pb->cblock = cblock; |
669 | pb->saved_bi_end_io = bio->bi_end_io; | 672 | pb->saved_bi_end_io = bio->bi_end_io; |
673 | dm_bio_record(&pb->bio_details, bio); | ||
670 | bio->bi_end_io = writethrough_endio; | 674 | bio->bi_end_io = writethrough_endio; |
671 | 675 | ||
672 | remap_to_origin_clear_discard(pb->cache, bio, oblock); | 676 | remap_to_origin_clear_discard(pb->cache, bio, oblock); |