From patchwork Mon Mar 25 10:25:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Thornber X-Patchwork-Id: 2330361 Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) by patchwork2.kernel.org (Postfix) with ESMTP id EB12EDF24C for ; Mon, 25 Mar 2013 10:28:32 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2PAP6Pl031793; Mon, 25 Mar 2013 06:25:07 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2PAP4D4005797 for ; Mon, 25 Mar 2013 06:25:04 -0400 Received: from localhost (vpn1-5-242.ams2.redhat.com [10.36.5.242]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2PAP1v0012129; Mon, 25 Mar 2013 06:25:03 -0400 Date: Mon, 25 Mar 2013 10:25:01 +0000 From: Joe Thornber To: dm-devel@redhat.com Message-ID: <20130325102500.GA6340@debian> Mail-Followup-To: dm-devel@redhat.com, agk@redhat.com MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-loop: dm-devel@redhat.com Cc: agk@redhat.com Subject: [dm-devel] dm-cache writethrough issue. X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com 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 Author: Joe Thornber 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. Tested-by: Darrick J. Wong --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel 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 @@ -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);