Re: [PATCH v2 1/2] btrfs: open code RCU for device name

[Wang Yugui <wangyugui@e16-tech.com>]

Hi,

> The RCU protected string is only used for a device name, and RCU is used
> so we can print the name and eventually synchronize against the rare
> device rename in device_list_add().
> 
> We don't need the whole API just for that. Open code all the helpers and
> access to the string itself.
> 
> Notable change is in device_list_add() when the device name is changed,
> which is the only place that can actually happen at the same time as
> message prints using the device name under RCU read lock.
> 
> Previously there was kfree_rcu() which used the embedded rcu_head to
> delay freeing the object depending on the RCU mechanism. Now there's
> kfree_rcu_mightsleep() which does not need the rcu_head and waits for
> the grace period.
> 
> Sleeping is safe in this context and as this is a rare event it won't
> interfere with the rest as it's holding the device_list_mutex.
> 
> Straightforward changes:
> 
> - rcu_string_strdup -> kstrdup
> - rcu_str_deref -> rcu_dereference
> - drop ->str from safe contexts
> 
> Historical notes:
> 
> Introduced in 606686eeac45 ("Btrfs: use rcu to protect device->name")
> with a vague reference of the potential problem described in
> https://lore.kernel.org/all/20120531155304.GF11775@ZenIV.linux.org.uk/ .
> 
> The RCU protection looks like the easiest and most lightweight way of
> protecting the rare event of device rename racing device_list_add()
> with a random printk() that uses the device name.
> 
> Alternatives: a spin lock would require to protect the printk
> anyway, a fixed buffer for the name would be eventually wrong in case
> the new name is overwritten when being printed, an array switching
> pointers and cleaning them up eventually resembles RCU too much.
> 
> The cleanups up to this patch should hide special case of RCU to the
> minimum that only the name needs rcu_dereference(), which can be further
> cleaned up to use btrfs_dev_name().
> 

There is still rcu warning when 'make  W=1 C=1'

/usr/hpc-bio/linux-6.12.35/fs/btrfs/volumes.c:405:21: warning: incorrect type in argument 1 (different address spaces)
/usr/hpc-bio/linux-6.12.35/fs/btrfs/volumes.c:405:21:    expected void const *objp
/usr/hpc-bio/linux-6.12.35/fs/btrfs/volumes.c:405:21:    got char const [noderef] __rcu *name

static void btrfs_free_device(struct btrfs_device *device)
{
    WARN_ON(!list_empty(&device->post_commit_list));
    /* No need to call kfree_rcu(), nothing is reading the device name. */
L405:    kfree(device->name);

do we need rcu_dereference here?
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -402,7 +402,7 @@ static void btrfs_free_device(struct btrfs_device *device)
 {
        WARN_ON(!list_empty(&device->post_commit_list));
        /* No need to call kfree_rcu(), nothing is reading the device name. */
-       kfree(device->name);
+       kfree(rcu_dereference(device->name));

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2025/06/30