| Message ID | 1447929317-2522-1-git-send-email-dsterba@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Thanks David. Checked other places it seems to be fine. My oversight on the rcu usage part. Thanks. Reviewed-by: Anand Jain <anand.jain@oracle.com> 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: [<ffffffffa00bc785>] btrfs_dev_replace_finishing+0x45/0xa00 [btrfs] > 1: (uuid_mutex){+.+.+.}, at: [<ffffffffa00bc84f>] btrfs_dev_replace_finishing+0x10f/0xa00 [btrfs] > 2: (&fs_devs->device_list_mutex){+.+.+.}, at: [<ffffffffa00bc868>] btrfs_dev_replace_finishing+0x128/0xa00 [btrfs] > 3: (&fs_info->chunk_mutex){+.+...}, at: [<ffffffffa00bc87d>] 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: > [<ffffffff8141d47b>] dump_stack+0x4f/0x74 > [<ffffffff810cd883>] lockdep_rcu_suspicious+0x103/0x140 > [<ffffffffa0071261>] btrfs_rm_dev_replace_remove_srcdev+0x111/0x130 [btrfs] > [<ffffffff810d354d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff81449536>] ? __percpu_counter_sum+0x66/0x80 > [<ffffffffa00bcc15>] btrfs_dev_replace_finishing+0x4d5/0xa00 [btrfs] > [<ffffffffa00bc96e>] ? btrfs_dev_replace_finishing+0x22e/0xa00 [btrfs] > [<ffffffffa00a8795>] ? btrfs_scrub_dev+0x415/0x6d0 [btrfs] > [<ffffffffa003ea69>] ? btrfs_start_transaction+0x9/0x20 [btrfs] > [<ffffffffa00bda79>] btrfs_dev_replace_start+0x339/0x590 [btrfs] > [<ffffffff81196aa5>] ? __might_fault+0x95/0xa0 > [<ffffffffa0078638>] btrfs_ioctl_dev_replace+0x118/0x160 [btrfs] > [<ffffffff811409c6>] ? stack_trace_call+0x46/0x70 > [<ffffffffa007c914>] ? btrfs_ioctl+0x24/0x1770 [btrfs] > [<ffffffffa007ce43>] btrfs_ioctl+0x553/0x1770 [btrfs] > [<ffffffff811409c6>] ? stack_trace_call+0x46/0x70 > [<ffffffff811d6eb1>] ? do_vfs_ioctl+0x21/0x5a0 > [<ffffffff811d6f1c>] do_vfs_ioctl+0x8c/0x5a0 > [<ffffffff811e3336>] ? __fget_light+0x86/0xb0 > [<ffffffff811e3369>] ? __fdget+0x9/0x20 > [<ffffffff811d7451>] ? SyS_ioctl+0x21/0x80 > [<ffffffff811d7483>] SyS_ioctl+0x53/0x80 > [<ffffffff81b1efd7>] 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 <dsterba@suse.com> > --- > 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--; > -- 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
=============================== [ 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: [<ffffffffa00bc785>] btrfs_dev_replace_finishing+0x45/0xa00 [btrfs] 1: (uuid_mutex){+.+.+.}, at: [<ffffffffa00bc84f>] btrfs_dev_replace_finishing+0x10f/0xa00 [btrfs] 2: (&fs_devs->device_list_mutex){+.+.+.}, at: [<ffffffffa00bc868>] btrfs_dev_replace_finishing+0x128/0xa00 [btrfs] 3: (&fs_info->chunk_mutex){+.+...}, at: [<ffffffffa00bc87d>] 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: [<ffffffff8141d47b>] dump_stack+0x4f/0x74 [<ffffffff810cd883>] lockdep_rcu_suspicious+0x103/0x140 [<ffffffffa0071261>] btrfs_rm_dev_replace_remove_srcdev+0x111/0x130 [btrfs] [<ffffffff810d354d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff81449536>] ? __percpu_counter_sum+0x66/0x80 [<ffffffffa00bcc15>] btrfs_dev_replace_finishing+0x4d5/0xa00 [btrfs] [<ffffffffa00bc96e>] ? btrfs_dev_replace_finishing+0x22e/0xa00 [btrfs] [<ffffffffa00a8795>] ? btrfs_scrub_dev+0x415/0x6d0 [btrfs] [<ffffffffa003ea69>] ? btrfs_start_transaction+0x9/0x20 [btrfs] [<ffffffffa00bda79>] btrfs_dev_replace_start+0x339/0x590 [btrfs] [<ffffffff81196aa5>] ? __might_fault+0x95/0xa0 [<ffffffffa0078638>] btrfs_ioctl_dev_replace+0x118/0x160 [btrfs] [<ffffffff811409c6>] ? stack_trace_call+0x46/0x70 [<ffffffffa007c914>] ? btrfs_ioctl+0x24/0x1770 [btrfs] [<ffffffffa007ce43>] btrfs_ioctl+0x553/0x1770 [btrfs] [<ffffffff811409c6>] ? stack_trace_call+0x46/0x70 [<ffffffff811d6eb1>] ? do_vfs_ioctl+0x21/0x5a0 [<ffffffff811d6f1c>] do_vfs_ioctl+0x8c/0x5a0 [<ffffffff811e3336>] ? __fget_light+0x86/0xb0 [<ffffffff811e3369>] ? __fdget+0x9/0x20 [<ffffffff811d7451>] ? SyS_ioctl+0x21/0x80 [<ffffffff811d7483>] SyS_ioctl+0x53/0x80 [<ffffffff81b1efd7>] 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 <dsterba@suse.com> --- 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--;