The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
@ 2026-08-20 12:27 Jiacheng Xu
  2026-08-20 22:32 ` Qu Wenruo
  2026-08-22  3:41 ` [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready Jiacheng Xu
  0 siblings, 2 replies; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-20 12:27 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Sterba, linux-btrfs, linux-kernel

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@zju.edu.cn>
Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
---
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);

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-20 12:27 [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread Jiacheng Xu
@ 2026-08-20 22:32 ` Qu Wenruo
  2026-08-21  4:30   ` Jiacheng Xu
  2026-08-22  3:41 ` [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready Jiacheng Xu
  1 sibling, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2026-08-20 22:32 UTC (permalink / raw)
  To: Jiacheng Xu, Chris Mason; +Cc: David Sterba, linux-btrfs, linux-kernel



在 2026/8/20 21:57, Jiacheng Xu 写道:
> 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.

Why not just simpliy reject sysfs write operations when the fs has 
CLOSING_START or without FS_OPEN flags?

> 
> Apply the same ordering to the open_ctree() failure path when the
> transaction kthread has already been created.
> 
> Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> ---
> 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);


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-20 22:32 ` Qu Wenruo
@ 2026-08-21  4:30   ` Jiacheng Xu
  2026-08-21  5:21     ` Qu Wenruo
  0 siblings, 1 reply; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-21  4:30 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel

Hi Wenruo,

I agree that rejecting sysfs writes when FS_OPEN is unset or
CLOSING_START is set fixes the mount-time NULL pointer dereference.

However, checking these flags alone does not fully protect the teardown
path. There is still a check-then-use race:

      sysfs store callback                close_ctree()

      test FS_OPEN == 1
      test CLOSING_START == 0

                                           set CLOSING_START
                                           kthread_stop(transaction_kthread)

      wake_up_process(transaction_kthread)
      use a stopped or freed task_struct

Thus, the flag check fixes the reported initialization race, but a
separate teardown race remains unless active sysfs callbacks are drained
or otherwise synchronized before stopping transaction_kthread.

Thanks,
Jiacheng

> -----原始邮件-----
> 发件人: "Qu Wenruo" <wqu@suse.com>
> 发送时间:2026-08-21 06:32:14 (星期五)
> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>, "Chris Mason" <clm@fb.com>
> 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
> 
> 
> 
> 在 2026/8/20 21:57, Jiacheng Xu 写道:
> > 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.
> 
> Why not just simpliy reject sysfs write operations when the fs has 
> CLOSING_START or without FS_OPEN flags?
> 
> > 
> > Apply the same ordering to the open_ctree() failure path when the
> > transaction kthread has already been created.
> > 
> > Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
> > Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> > ---
> > 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);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-21  4:30   ` Jiacheng Xu
@ 2026-08-21  5:21     ` Qu Wenruo
  2026-08-21  6:49       ` Jiacheng Xu
  0 siblings, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2026-08-21  5:21 UTC (permalink / raw)
  To: Jiacheng Xu; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel



在 2026/8/21 14:00, Jiacheng Xu 写道:
> Hi Wenruo,
> 
> I agree that rejecting sysfs writes when FS_OPEN is unset or
> CLOSING_START is set fixes the mount-time NULL pointer dereference.
> 
> However, checking these flags alone does not fully protect the teardown
> path. There is still a check-then-use race:
> 
>        sysfs store callback                close_ctree()
> 
>        test FS_OPEN == 1
>        test CLOSING_START == 0
> 
>                                             set CLOSING_START
>                                             kthread_stop(transaction_kthread)
> 
>        wake_up_process(transaction_kthread)
>        use a stopped or freed task_struct
> 
> Thus, the flag check fixes the reported initialization race, but a
> separate teardown race remains unless active sysfs callbacks are drained
> or otherwise synchronized before stopping transaction_kthread.

OK, then the next quesstion is, why we don't move the sysfs creation 
after the commit transaction creation.

Even with your patch, it didn't solve the problem that during mount the 
sysfs is created before transaction kthread.

So in theory it's possible to do sysfs write before kthread initialized, 
still causing NULL pointer dereference.

> 
> Thanks,
> Jiacheng
> 
>> -----原始邮件-----
>> 发件人: "Qu Wenruo" <wqu@suse.com>
>> 发送时间:2026-08-21 06:32:14 (星期五)
>> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>, "Chris Mason" <clm@fb.com>
>> 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
>> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
>>
>>
>>
>> 在 2026/8/20 21:57, Jiacheng Xu 写道:
>>> 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.
>>
>> Why not just simpliy reject sysfs write operations when the fs has
>> CLOSING_START or without FS_OPEN flags?
>>
>>>
>>> Apply the same ordering to the open_ctree() failure path when the
>>> transaction kthread has already been created.
>>>
>>> Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
>>> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
>>> ---
>>> 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);


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-21  5:21     ` Qu Wenruo
@ 2026-08-21  6:49       ` Jiacheng Xu
  2026-08-21  7:08         ` Qu Wenruo
  0 siblings, 1 reply; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-21  6:49 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel

I think you are right. The previous patch only addressed the teardown
case and did not fix the mount-time window.

However, moving the whole btrfs_sysfs_add_mounted() call is not a
simple reorder because btrfs_init_space_info() creates child kobjects
under space_info_kobj, which is created by btrfs_sysfs_add_mounted().

Also, creating sysfs after transaction_kthread but before BTRFS_FS_OPEN
would still expose sysfs writes while the mount is not fully initialized.

Do you have any ideas?

Thanks,
Jiacheng

> -----原始邮件-----
> 发件人: "Qu Wenruo" <wqu@suse.com>
> 发送时间:2026-08-21 13:21:42 (星期五)
> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>
> 抄送: "Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
> 
> 
> 
> 在 2026/8/21 14:00, Jiacheng Xu 写道:
> > Hi Wenruo,
> > 
> > I agree that rejecting sysfs writes when FS_OPEN is unset or
> > CLOSING_START is set fixes the mount-time NULL pointer dereference.
> > 
> > However, checking these flags alone does not fully protect the teardown
> > path. There is still a check-then-use race:
> > 
> >        sysfs store callback                close_ctree()
> > 
> >        test FS_OPEN == 1
> >        test CLOSING_START == 0
> > 
> >                                             set CLOSING_START
> >                                             kthread_stop(transaction_kthread)
> > 
> >        wake_up_process(transaction_kthread)
> >        use a stopped or freed task_struct
> > 
> > Thus, the flag check fixes the reported initialization race, but a
> > separate teardown race remains unless active sysfs callbacks are drained
> > or otherwise synchronized before stopping transaction_kthread.
> 
> OK, then the next quesstion is, why we don't move the sysfs creation 
> after the commit transaction creation.
> 
> Even with your patch, it didn't solve the problem that during mount the 
> sysfs is created before transaction kthread.
> 
> So in theory it's possible to do sysfs write before kthread initialized, 
> still causing NULL pointer dereference.
> 
> > 
> > Thanks,
> > Jiacheng
> > 
> >> -----原始邮件-----
> >> 发件人: "Qu Wenruo" <wqu@suse.com>
> >> 发送时间:2026-08-21 06:32:14 (星期五)
> >> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>, "Chris Mason" <clm@fb.com>
> >> 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> >> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
> >>
> >>
> >>
> >> 在 2026/8/20 21:57, Jiacheng Xu 写道:
> >>> 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.
> >>
> >> Why not just simpliy reject sysfs write operations when the fs has
> >> CLOSING_START or without FS_OPEN flags?
> >>
> >>>
> >>> Apply the same ordering to the open_ctree() failure path when the
> >>> transaction kthread has already been created.
> >>>
> >>> Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
> >>> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> >>> ---
> >>> 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);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-21  6:49       ` Jiacheng Xu
@ 2026-08-21  7:08         ` Qu Wenruo
  2026-08-21  9:05           ` Qu Wenruo
  0 siblings, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2026-08-21  7:08 UTC (permalink / raw)
  To: Jiacheng Xu; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel



在 2026/8/21 16:19, Jiacheng Xu 写道:
> I think you are right. The previous patch only addressed the teardown
> case and did not fix the mount-time window.
> 
> However, moving the whole btrfs_sysfs_add_mounted() call is not a
> simple reorder because btrfs_init_space_info() creates child kobjects
> under space_info_kobj, which is created by btrfs_sysfs_add_mounted().
> 
> Also, creating sysfs after transaction_kthread but before BTRFS_FS_OPEN
> would still expose sysfs writes while the mount is not fully initialized.
> 
> Do you have any ideas?

For the space_info kobj, I think we can de-couple space info and its 
kobj file creation.
Aka, allow btrfs_init_space_info() to do everything except the kobj 
creation.

Then at the very end, create every kobj needed, and at that time, the 
full fs should be fully initialized.

This should solve the problem from the root, but will definitely need 
quite some changes to the mount/unmount path.
> 
> Thanks,
> Jiacheng
> 
>> -----原始邮件-----
>> 发件人: "Qu Wenruo" <wqu@suse.com>
>> 发送时间:2026-08-21 13:21:42 (星期五)
>> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>
>> 抄送: "Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
>> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
>>
>>
>>
>> 在 2026/8/21 14:00, Jiacheng Xu 写道:
>>> Hi Wenruo,
>>>
>>> I agree that rejecting sysfs writes when FS_OPEN is unset or
>>> CLOSING_START is set fixes the mount-time NULL pointer dereference.
>>>
>>> However, checking these flags alone does not fully protect the teardown
>>> path. There is still a check-then-use race:
>>>
>>>         sysfs store callback                close_ctree()
>>>
>>>         test FS_OPEN == 1
>>>         test CLOSING_START == 0
>>>
>>>                                              set CLOSING_START
>>>                                              kthread_stop(transaction_kthread)
>>>
>>>         wake_up_process(transaction_kthread)
>>>         use a stopped or freed task_struct
>>>
>>> Thus, the flag check fixes the reported initialization race, but a
>>> separate teardown race remains unless active sysfs callbacks are drained
>>> or otherwise synchronized before stopping transaction_kthread.
>>
>> OK, then the next quesstion is, why we don't move the sysfs creation
>> after the commit transaction creation.
>>
>> Even with your patch, it didn't solve the problem that during mount the
>> sysfs is created before transaction kthread.
>>
>> So in theory it's possible to do sysfs write before kthread initialized,
>> still causing NULL pointer dereference.
>>
>>>
>>> Thanks,
>>> Jiacheng
>>>
>>>> -----原始邮件-----
>>>> 发件人: "Qu Wenruo" <wqu@suse.com>
>>>> 发送时间:2026-08-21 06:32:14 (星期五)
>>>> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>, "Chris Mason" <clm@fb.com>
>>>> 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
>>>> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
>>>>
>>>>
>>>>
>>>> 在 2026/8/20 21:57, Jiacheng Xu 写道:
>>>>> 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.
>>>>
>>>> Why not just simpliy reject sysfs write operations when the fs has
>>>> CLOSING_START or without FS_OPEN flags?
>>>>
>>>>>
>>>>> Apply the same ordering to the open_ctree() failure path when the
>>>>> transaction kthread has already been created.
>>>>>
>>>>> Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
>>>>> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
>>>>> ---
>>>>> 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);


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread
  2026-08-21  7:08         ` Qu Wenruo
@ 2026-08-21  9:05           ` Qu Wenruo
  0 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2026-08-21  9:05 UTC (permalink / raw)
  To: Jiacheng Xu; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel



在 2026/8/21 16:38, Qu Wenruo 写道:
> 
> 
> 在 2026/8/21 16:19, Jiacheng Xu 写道:
>> I think you are right. The previous patch only addressed the teardown
>> case and did not fix the mount-time window.
>>
>> However, moving the whole btrfs_sysfs_add_mounted() call is not a
>> simple reorder because btrfs_init_space_info() creates child kobjects
>> under space_info_kobj, which is created by btrfs_sysfs_add_mounted().
>>
>> Also, creating sysfs after transaction_kthread but before BTRFS_FS_OPEN
>> would still expose sysfs writes while the mount is not fully initialized.
>>
>> Do you have any ideas?
> 
> For the space_info kobj, I think we can de-couple space info and its 
> kobj file creation.
> Aka, allow btrfs_init_space_info() to do everything except the kobj 
> creation.
> 
> Then at the very end, create every kobj needed, and at that time, the 
> full fs should be fully initialized.

Or we can de-couple only the btrfs_attrs files creation from 
btrfs_sysfs_add_mounted(), so that at early stages we only create needed 
sub-directories.

Then at the end of the mount, after everything is properly set up, 
create btrfs_attrs files.

For umount it's the reverse, remove btrfs_attrs files before anything else.


I think this is smaller than de-coupling space info.

> 
> This should solve the problem from the root, but will definitely need 
> quite some changes to the mount/unmount path.
>>
>> Thanks,
>> Jiacheng
>>
>>> -----原始邮件-----
>>> 发件人: "Qu Wenruo" <wqu@suse.com>
>>> 发送时间:2026-08-21 13:21:42 (星期五)
>>> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>
>>> 抄送: "Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.com>, 
>>> linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
>>> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping 
>>> transaction kthread
>>>
>>>
>>>
>>> 在 2026/8/21 14:00, Jiacheng Xu 写道:
>>>> Hi Wenruo,
>>>>
>>>> I agree that rejecting sysfs writes when FS_OPEN is unset or
>>>> CLOSING_START is set fixes the mount-time NULL pointer dereference.
>>>>
>>>> However, checking these flags alone does not fully protect the teardown
>>>> path. There is still a check-then-use race:
>>>>
>>>>         sysfs store callback                close_ctree()
>>>>
>>>>         test FS_OPEN == 1
>>>>         test CLOSING_START == 0
>>>>
>>>>                                              set CLOSING_START
>>>>                                              
>>>> kthread_stop(transaction_kthread)
>>>>
>>>>         wake_up_process(transaction_kthread)
>>>>         use a stopped or freed task_struct
>>>>
>>>> Thus, the flag check fixes the reported initialization race, but a
>>>> separate teardown race remains unless active sysfs callbacks are 
>>>> drained
>>>> or otherwise synchronized before stopping transaction_kthread.
>>>
>>> OK, then the next quesstion is, why we don't move the sysfs creation
>>> after the commit transaction creation.
>>>
>>> Even with your patch, it didn't solve the problem that during mount the
>>> sysfs is created before transaction kthread.
>>>
>>> So in theory it's possible to do sysfs write before kthread initialized,
>>> still causing NULL pointer dereference.
>>>
>>>>
>>>> Thanks,
>>>> Jiacheng
>>>>
>>>>> -----原始邮件-----
>>>>> 发件人: "Qu Wenruo" <wqu@suse.com>
>>>>> 发送时间:2026-08-21 06:32:14 (星期五)
>>>>> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>, "Chris Mason" <clm@fb.com>
>>>>> 抄送: "David Sterba" <dsterba@suse.com>, linux- 
>>>>> btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
>>>>> 主题: Re: [PATCH] btrfs: drain sysfs callbacks before stopping 
>>>>> transaction kthread
>>>>>
>>>>>
>>>>>
>>>>> 在 2026/8/20 21:57, Jiacheng Xu 写道:
>>>>>> 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.
>>>>>
>>>>> Why not just simpliy reject sysfs write operations when the fs has
>>>>> CLOSING_START or without FS_OPEN flags?
>>>>>
>>>>>>
>>>>>> Apply the same ordering to the open_ctree() failure path when the
>>>>>> transaction kthread has already been created.
>>>>>>
>>>>>> Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
>>>>>> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
>>>>>> ---
>>>>>> 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);
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready
  2026-08-20 12:27 [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread Jiacheng Xu
  2026-08-20 22:32 ` Qu Wenruo
@ 2026-08-22  3:41 ` Jiacheng Xu
  2026-08-22  3:46   ` [PATCH v2 2/2] " Jiacheng Xu
  2026-08-22  4:57   ` [PATCH v2 0/2] " Qu Wenruo
  1 sibling, 2 replies; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-22  3:41 UTC (permalink / raw)
  To: wqu; +Cc: linux-btrfs, linux-kernel

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@zju.edu.cn>
> 发送时间:2026-08-20 20:27:23 (星期四)
> 收件人: "Chris Mason" <clm@fb.com>
> 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> 主题: [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@zju.edu.cn>
> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> ---
> 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);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] btrfs: delay mounted sysfs attributes until mount is ready
  2026-08-22  3:41 ` [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready Jiacheng Xu
@ 2026-08-22  3:46   ` Jiacheng Xu
  2026-08-22  4:57   ` [PATCH v2 0/2] " Qu Wenruo
  1 sibling, 0 replies; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-22  3:46 UTC (permalink / raw)
  To: wqu; +Cc: linux-btrfs, linux-kernel

btrfs_sysfs_add_mounted() publishes the writable label and feature
attributes before the transaction kthread is created. A concurrent sysfs
write can therefore reach wake_up_process() while
fs_info->transaction_kthread is still NULL and cause a null-ptr-dereference 
in try_to_wake_up().

A flag check in the store callbacks would not synchronize with teardown
after a callback has already passed the check. Instead, publish the fsid
attributes only after the transaction kthread and the rest of the mount
state are initialized. Do this before setting BTRFS_FS_OPEN so the cleaner
cannot update a feature group that has not been created yet.

On unmount, park the cleaner first so it cannot recreate the feature group.
Then remove the fsid attributes. The sysfs removal drains active callbacks
while both kthreads are still valid.

Fixes: a6f69dc8018d ("btrfs: move commit out of sysfs when changing label")
Fixes: 0eae2747ec1d ("btrfs: move commit out of sysfs when changing features")
Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
---
 fs/btrfs/disk-io.c | 18 +++++++++++++++++-
 fs/btrfs/sysfs.c   |  7 -------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 2f1666d..93d62a7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3747,8 +3747,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
                goto fail_qgroup;
        }
 
-       if (sb_rdonly(sb))
+       if (sb_rdonly(sb)) {
+               ret = btrfs_sysfs_add_mounted_attrs(fs_info);
+               if (ret)
+                       goto fail_qgroup;
                return 0;
+       }
 
        ret = btrfs_start_pre_rw_mount(fs_info);
        if (ret) {
@@ -3769,6 +3773,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
                }
        }
 
+       ret = btrfs_sysfs_add_mounted_attrs(fs_info);
+       if (ret) {
+               close_ctree(fs_info);
+               return ret;
+       }
+
        set_bit(BTRFS_FS_OPEN, &fs_info->flags);
 
        /* Kick the cleaner thread so it'll start deleting snapshots. */
@@ -4347,6 +4357,12 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
         */
        kthread_park(fs_info->cleaner_kthread);
 
+       /*
+        * The cleaner can no longer recreate feature attributes. Remove the
+        * fsid attributes and drain callbacks before stopping the kthreads.
+        */
+       btrfs_sysfs_remove_mounted_attrs(fs_info);
+
        /* wait for the qgroup rescan worker to stop */
        btrfs_qgroup_wait_for_completion(fs_info, false);
 
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index a89e5ae..5ecebb2 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1739,7 +1739,6 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
                kobject_put(fs_info->debug_kobj);
        }
 #endif
-       btrfs_sysfs_remove_mounted_attrs(fs_info);
        btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
 }
 
@@ -2325,12 +2324,6 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
        if (ret)
                return ret;
 
-       ret = btrfs_sysfs_add_mounted_attrs(fs_info);
-       if (ret) {
-               btrfs_sysfs_remove_fs_devices(fs_devs);
-               return ret;
-       }
-
 #ifdef CONFIG_BTRFS_DEBUG
        fs_info->debug_kobj = kobject_create_and_add("debug", fsid_kobj);
        if (!fs_info->debug_kobj) {
-- 
2.25.1


> -----原始邮件-----
> 发件人: "Jiacheng Xu" <stitch@zju.edu.cn>
> 发送时间:2026-08-22 11:41:15 (星期六)
> 收件人: wqu@suse.com
> 抄送: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> 主题: [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@zju.edu.cn>
> > 发送时间:2026-08-20 20:27:23 (星期四)
> > 收件人: "Chris Mason" <clm@fb.com>
> > 抄送: "David Sterba" <dsterba@suse.com>, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> > 主题: [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@zju.edu.cn>
> > Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> > ---
> > 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);

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready
  2026-08-22  3:41 ` [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready Jiacheng Xu
  2026-08-22  3:46   ` [PATCH v2 2/2] " Jiacheng Xu
@ 2026-08-22  4:57   ` Qu Wenruo
  2026-08-22  5:39     ` Jiacheng Xu
  1 sibling, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2026-08-22  4:57 UTC (permalink / raw)
  To: Jiacheng Xu; +Cc: linux-btrfs, linux-kernel



在 2026/8/22 13:11, Jiacheng Xu 写道:
> 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.

You don't need to bother feature attributes for now, there is already a 
patch addressing it by completely removing the write support for feature 
attributes:

https://lore.kernel.org/linux-btrfs/8a598d76555b5944d34bb08fa8dbeea28fc05db9.1787307129.git.wqu@suse.com/

Considering it's only extended_iref, removing it should be much simpler.
Until that is determined, you only need to bother the label one.


Furthermore, among all the attr files in the fsid directory, there is 
only label that is writable, it would make more sense to split 
btrfs_attrs into two parts, one for those read-only members, and one for 
the only writebale label one.

Otherwise the series looks much better.


> - 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Re: [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready
  2026-08-22  4:57   ` [PATCH v2 0/2] " Qu Wenruo
@ 2026-08-22  5:39     ` Jiacheng Xu
  0 siblings, 0 replies; 11+ messages in thread
From: Jiacheng Xu @ 2026-08-22  5:39 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, linux-kernel

Great! Please let me know if the patch is finally merged.

Thanks,
Jiacheng

> -----原始邮件-----
> 发件人: "Qu Wenruo" <wqu@suse.com>
> 发送时间:2026-08-22 12:57:14 (星期六)
> 收件人: "Jiacheng Xu" <stitch@zju.edu.cn>
> 抄送: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready
> 
> 
> 
> 在 2026/8/22 13:11, Jiacheng Xu 写道:
> > 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.
> 
> You don't need to bother feature attributes for now, there is already a 
> patch addressing it by completely removing the write support for feature 
> attributes:
> 
> https://lore.kernel.org/linux-btrfs/8a598d76555b5944d34bb08fa8dbeea28fc05db9.1787307129.git.wqu@suse.com/
> 
> Considering it's only extended_iref, removing it should be much simpler.
> Until that is determined, you only need to bother the label one.
> 
> 
> Furthermore, among all the attr files in the fsid directory, there is 
> only label that is writable, it would make more sense to split 
> btrfs_attrs into two parts, one for those read-only members, and one for 
> the only writebale label one.
> 
> Otherwise the series looks much better.
> 
> 
> > - 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-08-22  5:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 12:27 [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread Jiacheng Xu
2026-08-20 22:32 ` Qu Wenruo
2026-08-21  4:30   ` Jiacheng Xu
2026-08-21  5:21     ` Qu Wenruo
2026-08-21  6:49       ` Jiacheng Xu
2026-08-21  7:08         ` Qu Wenruo
2026-08-21  9:05           ` Qu Wenruo
2026-08-22  3:41 ` [PATCH v2 0/2] btrfs: delay mounted sysfs attributes until mount is ready Jiacheng Xu
2026-08-22  3:46   ` [PATCH v2 2/2] " Jiacheng Xu
2026-08-22  4:57   ` [PATCH v2 0/2] " Qu Wenruo
2026-08-22  5:39     ` Jiacheng Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox