From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cn.fujitsu.com ([59.151.112.132]:6899 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1753426AbaHLHWr (ORCPT ); Tue, 12 Aug 2014 03:22:47 -0400 Message-ID: <53E9C134.5020706@cn.fujitsu.com> Date: Tue, 12 Aug 2014 15:24:36 +0800 From: Miao Xie Reply-To: MIME-Version: 1.0 To: Anand Jain , Subject: Re: [PATCH 4/4] btrfs: update sprout seed pointer when seed fs is relinquished References: <1407750176-15853-1-git-send-email-anand.jain@oracle.com> <1407750176-15853-4-git-send-email-anand.jain@oracle.com> In-Reply-To: <1407750176-15853-4-git-send-email-anand.jain@oracle.com> Content-Type: text/plain; charset="UTF-8" Sender: linux-btrfs-owner@vger.kernel.org List-ID: On Mon, 11 Aug 2014 17:42:56 +0800, Anand Jain wrote: > We are not updating sprout fs seed pointer when all seed device > is replaced. This patch will check if all seed device has been > replaced and then update the sprout pointer accordingly. > > Same reproducer as in the previous patch would apply here. > And notice that btrfs_close_device will check if seed fs is > present and spits out the error with out this patch. > > int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > { > :: > seed_devices = fs_devices->seed; > :: > while (seed_devices) { > fs_devices = seed_devices; > seed_devices = fs_devices->seed; > __btrfs_close_devices(fs_devices); > free_fs_devices(fs_devices); > } > > Signed-off-by: Anand Jain > --- > fs/btrfs/volumes.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index f098ae7..bfdc11f 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1992,6 +1992,25 @@ void btrfs_rm_dev_replace_srcdev(struct btrfs_fs_info *fs_info, > btrfs_scratch_superblock(srcdev); > } > > + /* unless fs_devices is seed fs, num_devices shouldn't go > + * zero > + */ According to coding style, the preferred style for multi-line comments is(except files in net subsystem): /* * */ > + BUG_ON(!fs_devices->num_devices && !fs_devices->seeding); Use ASSERT? > + /* if this is no devs we rather delete the fs_devices */ > + if (!fs_devices->num_devices) { > + struct btrfs_fs_devices *tmp_fs_devices; > + > + tmp_fs_devices = fs_info->fs_devices; > + while (tmp_fs_devices) { > + if (tmp_fs_devices->seed == fs_devices) { > + tmp_fs_devices->seed = fs_devices->seed; > + break; > + } > + tmp_fs_devices = tmp_fs_devices->seed; > + } > + fs_devices->seed = NULL; Why not free fs_devices like btrfs_rm_device? The other is OK. Reviewed-by: Miao Xie > + } > call_rcu(&srcdev->rcu, free_device); > } > >