From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp1040.oracle.com ([141.146.126.69]:34337 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162031AbbKTItM (ORCPT ); Fri, 20 Nov 2015 03:49:12 -0500 Message-ID: <564EDE62.9070107@oracle.com> Date: Fri, 20 Nov 2015 16:48:34 +0800 From: Anand Jain MIME-Version: 1.0 To: David Sterba , linux-btrfs@vger.kernel.org Subject: Re: [PATCH for 4.4] btrfs: fix rcu warning during device replace References: <1447929317-2522-1-git-send-email-dsterba@suse.com> In-Reply-To: <1447929317-2522-1-git-send-email-dsterba@suse.com> Content-Type: text/plain; charset=windows-1252; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: Thanks David. Checked other places it seems to be fine. My oversight on the rcu usage part. Thanks. Reviewed-by: Anand Jain On 11/19/2015 06:35 PM, David Sterba wrote: > The test btrfs/011 triggers a rcu warning > > =============================== > [ INFO: suspicious RCU usage. ] > 4.4.0-rc1-default+ #286 Tainted: G W > ------------------------------- > fs/btrfs/volumes.c:1977 suspicious rcu_dereference_check() usage! > > other info that might help us debug this: > > rcu_scheduler_active = 1, debug_locks = 0 > 4 locks held by btrfs/28786: > > 0: (&fs_info->dev_replace.lock_finishing_cancel_unmount){+.+...}, at: [] btrfs_dev_replace_finishing+0x45/0xa00 [btrfs] > 1: (uuid_mutex){+.+.+.}, at: [] btrfs_dev_replace_finishing+0x10f/0xa00 [btrfs] > 2: (&fs_devs->device_list_mutex){+.+.+.}, at: [] btrfs_dev_replace_finishing+0x128/0xa00 [btrfs] > 3: (&fs_info->chunk_mutex){+.+...}, at: [] btrfs_dev_replace_finishing+0x13d/0xa00 [btrfs] > > stack backtrace: > CPU: 0 PID: 28786 Comm: btrfs Tainted: G W 4.4.0-rc1-default+ #286 > Hardware name: Intel Corporation SandyBridge Platform/To be filled by O.E.M., BIOS ASNBCPT1.86C.0031.B00.1006301607 06/30/2010 > 0000000000000001 ffff8800a07dfb48 ffffffff8141d47b 0000000000000001 > 0000000000000001 0000000000000000 ffff8801464a4f00 ffff8800a07dfb78 > ffffffff810cd883 ffff880146eb9400 ffff8800a3698600 ffff8800a33fe220 > Call Trace: > [] dump_stack+0x4f/0x74 > [] lockdep_rcu_suspicious+0x103/0x140 > [] btrfs_rm_dev_replace_remove_srcdev+0x111/0x130 [btrfs] > [] ? trace_hardirqs_on+0xd/0x10 > [] ? __percpu_counter_sum+0x66/0x80 > [] btrfs_dev_replace_finishing+0x4d5/0xa00 [btrfs] > [] ? btrfs_dev_replace_finishing+0x22e/0xa00 [btrfs] > [] ? btrfs_scrub_dev+0x415/0x6d0 [btrfs] > [] ? btrfs_start_transaction+0x9/0x20 [btrfs] > [] btrfs_dev_replace_start+0x339/0x590 [btrfs] > [] ? __might_fault+0x95/0xa0 > [] btrfs_ioctl_dev_replace+0x118/0x160 [btrfs] > [] ? stack_trace_call+0x46/0x70 > [] ? btrfs_ioctl+0x24/0x1770 [btrfs] > [] btrfs_ioctl+0x553/0x1770 [btrfs] > [] ? stack_trace_call+0x46/0x70 > [] ? do_vfs_ioctl+0x21/0x5a0 > [] do_vfs_ioctl+0x8c/0x5a0 > [] ? __fget_light+0x86/0xb0 > [] ? __fdget+0x9/0x20 > [] ? SyS_ioctl+0x21/0x80 > [] SyS_ioctl+0x53/0x80 > [] entry_SYSCALL_64_fastpath+0x12/0x6f > > This is because of unprotected use of rcu_dereference in > btrfs_scratch_superblocks. We can't add rcu locks around the whole > function because we read the superblock. > > The fix will use the rcu string buffer directly without the rcu locking. > Thi is safe as the device will not go away in the meantime. We're > holding the device list mutexes. > > Restructuring the code to narrow down the rcu section turned out to be > impossible, we need to call filp_open (through update_dev_time) on the > buffer and this could call kmalloc/__might_sleep. We could call kstrdup > with GFP_ATOMIC but it's not absolutely necessary. > > Fixes: 12b1c2637b6e (Btrfs: enhance btrfs_scratch_superblock to scratch all superblocks) > Signed-off-by: David Sterba > --- > fs/btrfs/volumes.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index a6df8fdc1312..cd425dfe059e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1973,8 +1973,7 @@ void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_fs_info *fs_info, > if (srcdev->writeable) { > fs_devices->rw_devices--; > /* zero out the old super if it is writable */ > - btrfs_scratch_superblocks(srcdev->bdev, > - rcu_str_deref(srcdev->name)); > + btrfs_scratch_superblocks(srcdev->bdev, srcdev->name->str); > } > > if (srcdev->bdev) > @@ -2024,8 +2023,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, > btrfs_sysfs_rm_device_link(fs_info->fs_devices, tgtdev); > > if (tgtdev->bdev) { > - btrfs_scratch_superblocks(tgtdev->bdev, > - rcu_str_deref(tgtdev->name)); > + btrfs_scratch_superblocks(tgtdev->bdev, tgtdev->name->str); > fs_info->fs_devices->open_devices--; > } > fs_info->fs_devices->num_devices--; >