From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx.ewheeler.net ([66.155.3.69]:50360 "EHLO mail.ewheeler.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932462AbdGKRmN (ORCPT ); Tue, 11 Jul 2017 13:42:13 -0400 Date: Tue, 11 Jul 2017 17:42:10 +0000 (UTC) From: Eric Wheeler To: Coly Li cc: Kai Krakow , linux-bcache@vger.kernel.org, linux-block@vger.kernel.org Subject: Re: [PATCH] bcache: only recovery I/O error for writethrough mode In-Reply-To: Message-ID: References: <20170710111828.97918-1-colyli@suse.de> <20170710234619.7388fde2@jupiter.sol.kaishome.de> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="1641074820-2093933934-1499794931=:17052" Sender: linux-block-owner@vger.kernel.org List-Id: linux-block@vger.kernel.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --1641074820-2093933934-1499794931=:17052 Content-Type: TEXT/PLAIN; charset=utf-8 Content-Transfer-Encoding: 8BIT On Tue, 11 Jul 2017, Coly Li wrote: > On 2017/7/11 上午5:46, Kai Krakow wrote: > > Am Mon, 10 Jul 2017 19:18:28 +0800 > > schrieb Coly Li : > > > >> If a read bio to cache device gets failed, bcache will try to > >> recovery it by forward the read bio to backing device. If backing > >> device responses read request successfully then the bio contains data > >> from backing device will be returned to uppper layer. > >> > >> The recovery effort in cached_dev_read_error() is not correct, and > >> there is report that corrupted data may returned when a dirty cache > >> device goes offline during reading I/O. > >> > >> For writeback cache mode, before dirty data are wrote back to backing > >> device, data blocks on backing device are not updated and consistent. > >> If a dirty cache device dropped and a read bio gets failed, bcache > >> will return its stale version from backing device. This is mistaken > >> behavior that applications don't expected, especially for data base > >> workload. > >> > >> This patch fixes the issue by only permit recoverable I/O when cached > >> device is in writethough mode, and s->recoverable is set. For other > >> cache mode, recovery I/O failure by reading backing device does not > >> make sense, bache just simply returns -EIO immediately. Since bcache keeps the dirty map in memory, can you allow the passthrough to the backing device if the block is clean, but -EIO otherwise? -- Eric Wheeler > > > > Shouldn't write-around mode be okay for the same reason, i.e. there's > > no stale version on disk? > > > > So the patch should read "mode != CACHE_MODE_WRITEBACK" (not sure about > > the constant), that would also match your textual description. > > > > For write-around mode, there is only backing device as bdev target, it > does not have such inconsistent data issue. > > And indeed, this is a try-best effort. I mean, if a cached device is > changed from writeback mode to writethrough mode, then the cache device > is offline with dirty data, data corruption will still be probably to > happen. Anyway, this patch fixes most of cases when people do not change > cache mode in runtime. > > Thanks. > > Coly > > > > >> Reported-by: Arne Wolf > >> Signed-off-by: Coly Li > >> --- > >> drivers/md/bcache/request.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c > >> index 019b3df9f1c6..6edacac9b00d 100644 > >> --- a/drivers/md/bcache/request.c > >> +++ b/drivers/md/bcache/request.c > >> @@ -702,8 +702,11 @@ static void cached_dev_read_error(struct closure > >> *cl) { > >> struct search *s = container_of(cl, struct search, cl); > >> struct bio *bio = &s->bio.bio; > >> + struct cached_dev *dc = container_of(s->d, struct > >> cached_dev, disk); > >> + unsigned mode = cache_mode(dc, NULL); > >> > >> - if (s->recoverable) { > >> + if (s->recoverable && > >> + (mode == CACHE_MODE_WRITETHROUGH)) { > >> /* Retry from the backing device: */ > >> trace_bcache_read_retry(s->orig_bio); > >> > > > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bcache" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --1641074820-2093933934-1499794931=:17052--