Commit b844fe69 authored by Darrick J. Wong's avatar Darrick J. Wong Committed by Alasdair G Kergon

dm cache: fix writes to cache device in writethrough mode

The dm-cache writethrough strategy introduced by commit e2e74d61
("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: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarJoe Thornber <ejt@redhat.com>
Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
Signed-off-by: default avatarAlasdair G Kergon <agk@redhat.com>
parent 07961ac7
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "dm.h" #include "dm.h"
#include "dm-bio-prison.h" #include "dm-bio-prison.h"
#include "dm-bio-record.h"
#include "dm-cache-metadata.h" #include "dm-cache-metadata.h"
#include <linux/dm-io.h> #include <linux/dm-io.h>
...@@ -205,6 +206,7 @@ struct per_bio_data { ...@@ -205,6 +206,7 @@ struct per_bio_data {
struct cache *cache; struct cache *cache;
dm_cblock_t cblock; dm_cblock_t cblock;
bio_end_io_t *saved_bi_end_io; bio_end_io_t *saved_bi_end_io;
struct dm_bio_details bio_details;
}; };
struct dm_cache_migration { struct dm_cache_migration {
...@@ -643,6 +645,7 @@ static void writethrough_endio(struct bio *bio, int err) ...@@ -643,6 +645,7 @@ static void writethrough_endio(struct bio *bio, int err)
return; return;
} }
dm_bio_restore(&pb->bio_details, bio);
remap_to_cache(pb->cache, bio, pb->cblock); remap_to_cache(pb->cache, bio, pb->cblock);
/* /*
...@@ -667,6 +670,7 @@ static void remap_to_origin_then_cache(struct cache *cache, struct bio *bio, ...@@ -667,6 +670,7 @@ static void remap_to_origin_then_cache(struct cache *cache, struct bio *bio,
pb->cache = cache; pb->cache = cache;
pb->cblock = cblock; pb->cblock = cblock;
pb->saved_bi_end_io = bio->bi_end_io; pb->saved_bi_end_io = bio->bi_end_io;
dm_bio_record(&pb->bio_details, bio);
bio->bi_end_io = writethrough_endio; bio->bi_end_io = writethrough_endio;
remap_to_origin_clear_discard(pb->cache, bio, oblock); remap_to_origin_clear_discard(pb->cache, bio, oblock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment