From patchwork Thu Jul 4 12:20:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Behrens X-Patchwork-Id: 2822941 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 73872BF4A1 for ; Thu, 4 Jul 2013 12:21:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6963E20148 for ; Thu, 4 Jul 2013 12:21:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C2C6820143 for ; Thu, 4 Jul 2013 12:21:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756575Ab3GDMU7 (ORCPT ); Thu, 4 Jul 2013 08:20:59 -0400 Received: from mo-p00-ob.rzone.de ([81.169.146.162]:57515 "EHLO mo-p00-ob.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756483Ab3GDMU7 (ORCPT ); Thu, 4 Jul 2013 08:20:59 -0400 X-RZG-AUTH: :IGUKYFjleetgZuRbHZjp6Ve7NzeE1efWuTR/wV06y353QgIuD5+acdRFtJ8MDHZp4u74mTVB5dOWYQ== X-RZG-CLASS-ID: mo00 Received: from [172.24.1.80] (yian-ho01.nir.cronon.net [192.166.201.94]) by smtp.strato.de (josoe mo46) (RZmta 31.29 AUTH) with ESMTPA id D03e17p64CIcUv ; Thu, 4 Jul 2013 14:20:56 +0200 (CEST) Message-ID: <51D568A9.8030709@giantdisaster.de> Date: Thu, 04 Jul 2013 14:20:57 +0200 From: Stefan Behrens User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 MIME-Version: 1.0 To: Miao Xie CC: linux-btrfs@vger.kernel.org Subject: Re: [PATCH] Btrfs: fix wrong write offset when replacing a device References: <1372934241-421-1-git-send-email-miaox@cn.fujitsu.com> In-Reply-To: <1372934241-421-1-git-send-email-miaox@cn.fujitsu.com> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 4 Jul 2013 18:37:21 +0800, Miao Xie wrote: > The filesystem was corrupted after we did a device replace. > > Steps to reproduce: > # mkfs.btrfs -f -m single -d raid10 .. > # mount > # btrfs replace start -rfB 1 > # umount > # btrfsck > > The reason is that we changed the write offset by mistake. When we > replace the a device, we read the data from the source device at first, > and then write the data into the corresponding place of the new device. > But when we read the data, we will try to spread the read request among > the mirrors. It is a good idea that can speed up the read request, but > should not change the write offset because it is corresponding to the source > device, not the mirror device, if we change it by the offset of the mirror > device, we would get the wrong offset. Fix it. The main reason for the remapping is not to try to spread the read request, it is done in order to implement the "-r" option ("-r only read from srcdev if no other zero-defect mirror exists", this handles the case where the srcdev is very slow due to lots of read errors). Commit 625f1c8dc changed the write address. It was the right address before, the unmapped one. The physical disk address that is used to read from disk needs to be the mapped one (refer to scrub_add_page_to_rd_bio()), the address to write to disk needs to be the unmapped one. With the current patch, you use the right one for writing but the wrong one for reading. The following code should fix the wrong write address, it reverts the change to the write address from the aforementioned commit. I'll prepare and send a proper commit (after testing all RAID/single/dup cases). > > Signed-off-by: Miao Xie > --- > fs/btrfs/scrub.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 929093d..092bec4 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -228,7 +228,6 @@ static void scrub_bio_end_io_worker(struct btrfs_work *work); > static void scrub_block_complete(struct scrub_block *sblock); > static void scrub_remap_extent(struct btrfs_fs_info *fs_info, > u64 extent_logical, u64 extent_len, > - u64 *extent_physical, > struct btrfs_device **extent_dev, > int *extent_mirror_num); > static int scrub_setup_wr_ctx(struct scrub_ctx *sctx, > @@ -2482,8 +2481,7 @@ again: > extent_mirror_num = mirror_num; > if (is_dev_replace) > scrub_remap_extent(fs_info, extent_logical, > - extent_len, &extent_physical, > - &extent_dev, > + extent_len, &extent_dev, > &extent_mirror_num); > > ret = btrfs_lookup_csums_range(csum_root, logical, > @@ -3057,7 +3055,6 @@ int btrfs_scrub_progress(struct btrfs_root *root, u64 devid, > > static void scrub_remap_extent(struct btrfs_fs_info *fs_info, > u64 extent_logical, u64 extent_len, > - u64 *extent_physical, > struct btrfs_device **extent_dev, > int *extent_mirror_num) > { > @@ -3074,7 +3071,6 @@ static void scrub_remap_extent(struct btrfs_fs_info *fs_info, > return; > } > > - *extent_physical = bbio->stripes[0].physical; > *extent_mirror_num = bbio->mirror_num; > *extent_dev = bbio->stripes[0].dev; > kfree(bbio); > --- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2495,7 +2495,8 @@ again: ret = scrub_extent(sctx, extent_logical, extent_len, extent_physical, extent_dev, flags, generation, extent_mirror_num, - extent_physical); + extent_logical - logical + + physical); if (ret) goto out;