[PATCH v2 1/2] btrfs: sysfs: factor out mounted fsid attribute

[Jiacheng Xu]

The attributes attached directly to the mounted filesystem fsid kobject
are currently created and removed as part of the broader mounted sysfs
setup.

Factor the main filesystem attributes, the feature group and dynamically
generated unknown feature attributes into dedicated helpers. Keep the
helpers called from the existing mounted sysfs setup and teardown paths for
now. This prepares for adjusting their lifetime separately from the
required subdirectories.

Signed-off-by: Jiacheng Xu <stitch@xxxxxxxxxx>
---
 fs/btrfs/sysfs.c | 47 +++++++++++++++++++++++++++++++++++------------
 fs/btrfs/sysfs.h |  2 ++
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 0d14570..a89e5ae 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1707,6 +1707,15 @@ static void btrfs_sysfs_remove_fs_devices(struct btrfs_fs_devices *fs_devices)
        }
 }
 
+void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info)
+{
+       struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
+
+       addrm_unknown_feature_attrs(fs_info, false);
+       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
+       sysfs_remove_files(fsid_kobj, btrfs_attrs);
+}
+
 void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 {
        struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
@@ -1730,9 +1739,7 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
                kobject_put(fs_info->debug_kobj);
        }
 #endif
-       addrm_unknown_feature_attrs(fs_info, false);
-       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
-       sysfs_remove_files(fsid_kobj, btrfs_attrs);
+       btrfs_sysfs_remove_mounted_attrs(fs_info);
        btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
 }
 
@@ -2284,6 +2291,30 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
        return 0;
 }
 
+int btrfs_sysfs_add_mounted_attrs(struct btrfs_fs_info *fs_info)
+{
+       struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
+       int ret;
+
+       ret = sysfs_create_files(fsid_kobj, btrfs_attrs);
+       if (ret)
+               return ret;
+
+       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
+       if (ret)
+               goto failure;
+
+       ret = addrm_unknown_feature_attrs(fs_info, true);
+       if (ret)
+               goto failure;
+
+       return 0;
+
+failure:
+       btrfs_sysfs_remove_mounted_attrs(fs_info);
+       return ret;
+}
+
 int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 {
        int ret;
@@ -2294,16 +2325,12 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
        if (ret)
                return ret;
 
-       ret = sysfs_create_files(fsid_kobj, btrfs_attrs);
+       ret = btrfs_sysfs_add_mounted_attrs(fs_info);
        if (ret) {
                btrfs_sysfs_remove_fs_devices(fs_devs);
                return ret;
        }
 
-       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
-       if (ret)
-               goto failure;
-
 #ifdef CONFIG_BTRFS_DEBUG
        fs_info->debug_kobj = kobject_create_and_add("debug", fsid_kobj);
        if (!fs_info->debug_kobj) {
@@ -2327,10 +2354,6 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
        if (ret)
                goto failure;
 
-       ret = addrm_unknown_feature_attrs(fs_info, true);
-       if (ret)
-               goto failure;
-
        ret = sysfs_create_link(fsid_kobj, &fs_info->sb->s_bdi->dev->kobj, "bdi");
        if (ret)
                goto failure;
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 05498e5..d71438f 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -35,6 +35,8 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
 int __init btrfs_init_sysfs(void);
 void __cold btrfs_exit_sysfs(void);
 int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info);
+int btrfs_sysfs_add_mounted_attrs(struct btrfs_fs_info *fs_info);
+void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_add_block_group_type(struct btrfs_block_group *cache);
 int btrfs_sysfs_add_space_info_type(struct btrfs_space_info *space_info);
-- 
2.25.1


> -----原始邮件-----
> 发件人: "Jiacheng Xu" <stitch@xxxxxxxxxx>
> 发送时间:2026-08-22 11:41:15 (星期六)
> 收件人: wqu@xxxxxxxx
> 抄送: linux-btrfs@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> 主题: [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready
> 
> Here is a potential fix following Wenruo's idea.
> 
> btrfs_sysfs_add_mounted() currently publishes the writable label and
> feature attributes before the transaction kthread is created. A concurrent
> sysfs write can therefore dereference a NULL transaction_kthread in
> wake_up_process().
> 
> This series follows the suggested lifecycle: create only the required
> subdirectories during early mount, publish the fsid attributes after mount
> initialization, and remove them before the kthreads are stopped. The
> feature attributes are included because their store callback has the same
> transaction_kthread dependency as the label callback.
> 
> Patch 1 factors the fsid attribute handling into dedicated helpers. Patch 2
> moves their publication and removal to the safe mount and unmount stages.
> On unmount the cleaner is parked before attribute removal so it cannot
> recreate the feature group through sysfs_update_group(). Both patches are
> required for stable backports.
> 
> The resulting fs/btrfs/sysfs.o and fs/btrfs/disk-io.o were build-tested.
> 
> Changes in v2:
> - Delay creation of both the root and feature attributes until mount setup
>   is complete.
> - Remove those attributes while their kthread dependencies are still
>   valid.
> - Split helper extraction from the lifecycle fix for stable backports.
> 
> Jiacheng Xu (2):
>   btrfs: sysfs: factor out mounted fsid attribute helpers
>   btrfs: delay mounted fsid attributes until the fs is ready
> 
>  fs/btrfs/disk-io.c | 18 ++++++++++++++++-
>  fs/btrfs/sysfs.c   | 50 ++++++++++++++++++++++++++++++----------------
>  fs/btrfs/sysfs.h   |  2 ++
>  3 files changed, 52 insertions(+), 18 deletions(-)
> 
> base-commit: 0f23d56f17fdfc7db69d51f64c8b91bbab947aa9
> -- 
> 2.25.1
> 
> 
> > -----原始邮件-----
> > 发件人: "Jiacheng Xu" <stitch@xxxxxxxxxx>
> > 发送时间:2026-08-20 20:27:23 (星期四)
> > 收件人: "Chris Mason" <clm@xxxxxx>
> > 抄送: "David Sterba" <dsterba@xxxxxxxx>, linux-btrfs@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> > 主题: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
> > 
> > btrfs_label_store() and btrfs_feature_attr_store() wake up the
> > transaction kthread through fs_info->transaction_kthread.
> > 
> > During filesystem teardown, close_ctree() stops the transaction kthread
> > before removing the mounted filesystem's sysfs attributes. A concurrent
> > sysfs write can therefore enter one of these callbacks after the kthread
> > has been stopped and pass an invalid task pointer to wake_up_process().
> > 
> > This results in a concurrent null-pointer dereference in
> > try_to_wake_up(). The scheduler is not the root cause; the invalid
> > transaction kthread pointer is used by a Btrfs sysfs callback during
> > teardown.
> > 
> > Split mounted sysfs cleanup into two stages. Remove attributes which may
> > have store callbacks before stopping the transaction kthread. The
> > remaining sysfs kobjects are removed at the original teardown point,
> > after the kthread has been stopped.
> > 
> > Apply the same ordering to the open_ctree() failure path when the
> > transaction kthread has already been created.
> > 
> > Tested-by: Jiacheng Xu <stitch@xxxxxxxxxx>
> > Signed-off-by: Jiacheng Xu <stitch@xxxxxxxxxx>
> > ---
> > fs/btrfs/disk-io.c | 16 ++++++++++++++--
> > fs/btrfs/sysfs.c   | 26 +++++++++++++++++++++-----
> > fs/btrfs/sysfs.h   |  3 +++
> > 3 files changed, 38 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 2f1666d9544e..4f5bcc576dc6 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -3363,6 +3363,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> >       struct btrfs_root *tree_root;
> >       struct btrfs_root *chunk_root;
> >       struct btrfs_root *remap_root;
> > +     bool sysfs_attrs_removed = false;
> >       int ret;
> >       int level;
> > 
> > @@ -3780,6 +3781,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> > fail_qgroup:
> >       btrfs_free_qgroup_config(fs_info);
> > fail_trans_kthread:
> > +     btrfs_sysfs_remove_mounted_attrs(fs_info);
> > +     sysfs_attrs_removed = true;
> > +
> >       kthread_stop(fs_info->transaction_kthread);
> >       btrfs_cleanup_transaction(fs_info);
> >       btrfs_free_fs_roots(fs_info);
> > @@ -3793,7 +3797,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> >       filemap_write_and_wait(fs_info->btree_inode->i_mapping);
> > 
> > fail_sysfs:
> > -     btrfs_sysfs_remove_mounted(fs_info);
> > +     if (!sysfs_attrs_removed)
> > +             btrfs_sysfs_remove_mounted_attrs(fs_info);
> > +     btrfs_sysfs_remove_mounted_kobjects(fs_info);
> > 
> > fail_fsdev_sysfs:
> >       btrfs_sysfs_remove_fsid(fs_info->fs_devices);
> > @@ -4318,6 +4324,9 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> > 
> >       set_bit(BTRFS_FS_CLOSING_START, &fs_info->flags);
> > 
> > +     /* Drain sysfs callbacks before stopping the transaction kthread. */
> > +     btrfs_sysfs_remove_mounted_attrs(fs_info);
> > +
> >       /*
> >       * If we had UNFINISHED_DROPS we could still be processing them, so
> >       * clear that bit and wake up relocation so it can stop.
> > @@ -4538,7 +4547,7 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> >                         percpu_counter_sum(&fs_info->ordered_bytes));
> > 
> > -     btrfs_sysfs_remove_mounted(fs_info);
> > +     btrfs_sysfs_remove_mounted_kobjects(fs_info);
> >       btrfs_sysfs_remove_fsid(fs_info->fs_devices);
> > 
> >       btrfs_put_block_group_cache(fs_info);
> > 
> > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> > index 0d14570c8bc2..d90d76a152e9 100644
> > --- a/fs/btrfs/sysfs.c
> > +++ b/fs/btrfs/sysfs.c
> > @@ -1707,11 +1707,23 @@ static void btrfs_sysfs_remove_fs_devices(struct btrfs_fs_devices *fs_devices)
> >       }
> > }
> > 
> > -void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> > +/*
> > + * Remove attributes which may have store callbacks. kernfs waits for active
> > + * callbacks during removal, so this must be done before stopping any kthread
> > + * which can be woken up by those callbacks.
> > + */
> > +void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info)
> > {
> >       struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
> > 
> > -     sysfs_remove_link(fsid_kobj, "bdi");
> > +     addrm_unknown_feature_attrs(fs_info, false);
> > +     sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> > +     sysfs_remove_files(fsid_kobj, btrfs_attrs);
> > +}
> > +
> > +static void btrfs_sysfs_remove_mounted_dirs(struct btrfs_fs_info *fs_info)
> > +{
> > +     sysfs_remove_link(&fs_info->fs_devices->fsid_kobj, "bdi");
> > 
> >       if (fs_info->space_info_kobj) {
> >             sysfs_remove_files(fs_info->space_info_kobj, allocation_attrs);
> > @@ -1730,9 +1742,18 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> >             kobject_put(fs_info->debug_kobj);
> >       }
> > #endif
> > -     addrm_unknown_feature_attrs(fs_info, false);
> > -     sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> > -     sysfs_remove_files(fsid_kobj, btrfs_attrs);
> > +}
> > +
> > +void btrfs_sysfs_remove_mounted_kobjects(struct btrfs_fs_info *fs_info)
> > +{
> > +     btrfs_sysfs_remove_mounted_dirs(fs_info);
> > +     btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
> > +}
> > +
> > +void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> > +{
> > +     btrfs_sysfs_remove_mounted_dirs(fs_info);
> > +     btrfs_sysfs_remove_mounted_attrs(fs_info);
> >       btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
> > }
> > 
> > diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> > index 05498e5346c3..0d008fc8f1b8 100644
> > --- a/fs/btrfs/sysfs.h
> > +++ b/fs/btrfs/sysfs.h
> > @@ -35,6 +35,9 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
> > int __init btrfs_init_sysfs(void);
> > void __cold btrfs_exit_sysfs(void);
> > int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info);
> > +void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info);
> > +void btrfs_sysfs_remove_mounted_kobjects(struct btrfs_fs_info *fs_info);
> > void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
> > void btrfs_sysfs_add_block_group_type(struct btrfs_block_group *cache);
> > int btrfs_sysfs_add_space_info_type(struct btrfs_space_info *space_info);