* [PATCH for 4.4] btrfs: fix rcu warning during device replace
@ 2015-11-19 10:35 David Sterba
2015-11-20 8:48 ` Anand Jain
0 siblings, 1 reply; 2+ messages in thread
From: David Sterba @ 2015-11-19 10:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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--;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH for 4.4] btrfs: fix rcu warning during device replace
2015-11-19 10:35 [PATCH for 4.4] btrfs: fix rcu warning during device replace David Sterba
@ 2015-11-20 8:48 ` Anand Jain
0 siblings, 0 replies; 2+ messages in thread
From: Anand Jain @ 2015-11-20 8:48 UTC (permalink / raw)
To: David Sterba, linux-btrfs
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--;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-11-20 8:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-19 10:35 [PATCH for 4.4] btrfs: fix rcu warning during device replace David Sterba
2015-11-20 8:48 ` Anand Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox