[dm-devel] dm-cache writethrough issue.

[Darrick J. Wong]

On Mon, Mar 25, 2013 at 10:25:01AM +0000, Joe Thornber wrote:
> Alasdair,
> 
> Could you push this upstream ASAP please?
> 
> I'm not happy that we're now adding such a large structure to the per
> bio data, at the very least we should only do this if writethrough is
> enabled.  But I'll improve this later, for now correctness is the
> important thing.
> 
> - Joe

Makes my problems go away on my setup, so you can add:
Tested-by: Darrick J. Wong <darrick.wong at oracle.com>

--D
> 
> 
> 
> Author: Joe Thornber <ejt at redhat.com>
> Date:   Mon Mar 25 10:10:58 2013 +0000
> 
>     [dm-cache] Writethrough mode wasn't resetting bio fields before reusing
>     
>     The writethrough mode reuses the same bio to write to both the origin
>     and cache device.  It was recording the bi_sector and restoring this,
>     but this was inadequate; lower level drivers can change all sorts of
>     bio fields.
>     
>     This patch adds a struct dm_bio_details field, and uses
>     dm_bio_record() and dm_bio_restore() to ensure the bio is restored
>     before reissuing to the cache device.
>     
>     Issue discovered by Darrick Wong.
> 
> 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);
> 
> --
> dm-devel mailing list
> dm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel