btrfs: Fix suspicious RCU usage warning in device_list_add
diff mbox series

Message ID 20181114072456.4746-1-lufq.fnst@cn.fujitsu.com
State New
Headers show
Series
  • btrfs: Fix suspicious RCU usage warning in device_list_add
Related show

Commit Message

Lu Fengqi Nov. 14, 2018, 7:24 a.m. UTC

Comments

Nikolay Borisov Nov. 14, 2018, 7:30 a.m. UTC | #1
On 14.11.18 г. 9:24 ч., Lu Fengqi wrote:
> =============================
> WARNING: suspicious RCU usage
> 4.20.0-rc2+ #23 Tainted: G           O
> -----------------------------
> fs/btrfs/volumes.c:886 suspicious rcu_dereference_check() usage!
> 
> Use btrfs_info_in_rcu instead of pr_info for the required lock/unlock of
> RCU string.
> 
> Fixes: 1f265fc6f58b ("btrfs: harden agaist duplicate fsid on scanned devices")
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/volumes.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2186300bab91..6039ae5c549e 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -873,15 +873,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  			if (device->bdev != path_bdev) {
>  				bdput(path_bdev);
>  				mutex_unlock(&fs_devices->device_list_mutex);
> -				pr_warn(
> -		"BTRFS: duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
> +				btrfs_warn_in_rcu(device->fs_info,
> +			"duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
>  					disk_super->fsid, devid,
>  					rcu_str_deref(device->name), path);
>  				return ERR_PTR(-EEXIST);
>  			}
>  			bdput(path_bdev);
> -			pr_info(
> -			"BTRFS: device fsid %pU devid %llu moved old:%s new:%s\n",
> +			btrfs_info_in_rcu(device->fs_info,
> +			"device fsid %pU devid %llu moved old:%s new:%s\n",
>  				disk_super->fsid, devid,
>  				rcu_str_deref(device->name), path);
>  		}
>
Anand Jain Nov. 14, 2018, 1:19 p.m. UTC | #2
On 11/14/2018 03:24 PM, Lu Fengqi wrote:
> =============================
> WARNING: suspicious RCU usage
> 4.20.0-rc2+ #23 Tainted: G           O
> -----------------------------
> fs/btrfs/volumes.c:886 suspicious rcu_dereference_check() usage!
> 
> Use btrfs_info_in_rcu instead of pr_info for the required lock/unlock of
> RCU string.
> 
> Fixes: 1f265fc6f58b ("btrfs: harden agaist duplicate fsid on scanned devices")
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

  Thanks Lu. I missed it.
  Reviewed-by: Anand Jain <anand.jain@oracle.com>


> ---
>   fs/btrfs/volumes.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2186300bab91..6039ae5c549e 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -873,15 +873,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>   			if (device->bdev != path_bdev) {
>   				bdput(path_bdev);
>   				mutex_unlock(&fs_devices->device_list_mutex);
> -				pr_warn(
> -		"BTRFS: duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
> +				btrfs_warn_in_rcu(device->fs_info,
> +			"duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
>   					disk_super->fsid, devid,
>   					rcu_str_deref(device->name), path);
>   				return ERR_PTR(-EEXIST);
>   			}
>   			bdput(path_bdev);
> -			pr_info(
> -			"BTRFS: device fsid %pU devid %llu moved old:%s new:%s\n",
> +			btrfs_info_in_rcu(device->fs_info,
> +			"device fsid %pU devid %llu moved old:%s new:%s\n",
>   				disk_super->fsid, devid,
>   				rcu_str_deref(device->name), path);
>   		}
>
David Sterba Nov. 14, 2018, 4:05 p.m. UTC | #3
On Wed, Nov 14, 2018 at 03:24:56PM +0800, Lu Fengqi wrote:
> =============================
> WARNING: suspicious RCU usage
> 4.20.0-rc2+ #23 Tainted: G           O
> -----------------------------
> fs/btrfs/volumes.c:886 suspicious rcu_dereference_check() usage!
> 
> Use btrfs_info_in_rcu instead of pr_info for the required lock/unlock of
> RCU string.
> 
> Fixes: 1f265fc6f58b ("btrfs: harden agaist duplicate fsid on scanned devices")

Thanks for the fix.

Please note that the patch is still in the devel queue (misc-next) so
the commit id is unstable, and such fixups get folded to the patch.

You may also reply to the original mail with patch, but sending a bare
code change without a full changelog is also fine if the original patch
was sent long time ago and the fixup could get lost.

> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  fs/btrfs/volumes.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2186300bab91..6039ae5c549e 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -873,15 +873,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  			if (device->bdev != path_bdev) {
>  				bdput(path_bdev);
>  				mutex_unlock(&fs_devices->device_list_mutex);
> -				pr_warn(
> -		"BTRFS: duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
> +				btrfs_warn_in_rcu(device->fs_info,
> +			"duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",

The trailing newline is appended by all btrfs_* message helpers, removed
in the commit.
Lu Fengqi Nov. 15, 2018, 1:21 a.m. UTC | #4
On Wed, Nov 14, 2018 at 05:05:48PM +0100, David Sterba wrote:
>On Wed, Nov 14, 2018 at 03:24:56PM +0800, Lu Fengqi wrote:
>> =============================
>> WARNING: suspicious RCU usage
>> 4.20.0-rc2+ #23 Tainted: G           O
>> -----------------------------
>> fs/btrfs/volumes.c:886 suspicious rcu_dereference_check() usage!
>> 
>> Use btrfs_info_in_rcu instead of pr_info for the required lock/unlock of
>> RCU string.
>> 
>> Fixes: 1f265fc6f58b ("btrfs: harden agaist duplicate fsid on scanned devices")
>
>Thanks for the fix.
>
>Please note that the patch is still in the devel queue (misc-next) so
>the commit id is unstable, and such fixups get folded to the patch.
>
>You may also reply to the original mail with patch, but sending a bare
>code change without a full changelog is also fine if the original patch
>was sent long time ago and the fixup could get lost.

Got it.

Patch
diff mbox series

=============================
WARNING: suspicious RCU usage
4.20.0-rc2+ #23 Tainted: G           O
-----------------------------
fs/btrfs/volumes.c:886 suspicious rcu_dereference_check() usage!

Use btrfs_info_in_rcu instead of pr_info for the required lock/unlock of
RCU string.

Fixes: 1f265fc6f58b ("btrfs: harden agaist duplicate fsid on scanned devices")
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2186300bab91..6039ae5c549e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -873,15 +873,15 @@  static noinline struct btrfs_device *device_list_add(const char *path,
 			if (device->bdev != path_bdev) {
 				bdput(path_bdev);
 				mutex_unlock(&fs_devices->device_list_mutex);
-				pr_warn(
-		"BTRFS: duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
+				btrfs_warn_in_rcu(device->fs_info,
+			"duplicate device fsid:devid for %pU:%llu old:%s new:%s\n",
 					disk_super->fsid, devid,
 					rcu_str_deref(device->name), path);
 				return ERR_PTR(-EEXIST);
 			}
 			bdput(path_bdev);
-			pr_info(
-			"BTRFS: device fsid %pU devid %llu moved old:%s new:%s\n",
+			btrfs_info_in_rcu(device->fs_info,
+			"device fsid %pU devid %llu moved old:%s new:%s\n",
 				disk_super->fsid, devid,
 				rcu_str_deref(device->name), path);
 		}