Re: [PATCH 03/31] fuse: make debugging configurable at runtime

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 21, 2026 at 03:42:04PM -0800, Joanne Koong wrote:
> On Tue, Oct 28, 2025 at 5:45 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Use static keys so that we can configure debugging assertions and dmesg
> > warnings at runtime.  By default this is turned off so the cost is
> > merely scanning a nop sled.  However, fuse server developers can turn
> > it on for their debugging systems.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  fs/fuse/fuse_i.h     |    8 +++++
> >  fs/fuse/iomap_i.h    |   16 ++++++++--
> >  fs/fuse/Kconfig      |   15 +++++++++
> >  fs/fuse/file_iomap.c |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/fuse/inode.c      |    7 ++++
> >  5 files changed, 124 insertions(+), 3 deletions(-)
> >
> >
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 45be59df7ae592..61fb65f3604d61 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1691,6 +1691,14 @@ extern void fuse_sysctl_unregister(void);
> >  #define fuse_sysctl_unregister()       do { } while (0)
> >  #endif /* CONFIG_SYSCTL */
> >
> > +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> > +int fuse_iomap_sysfs_init(struct kobject *kobj);
> > +void fuse_iomap_sysfs_cleanup(struct kobject *kobj);
> > +#else
> > +# define fuse_iomap_sysfs_init(...)            (0)
> > +# define fuse_iomap_sysfs_cleanup(...)         ((void)0)
> > +#endif
> > +
> >  #if IS_ENABLED(CONFIG_FUSE_IOMAP)
> >  bool fuse_iomap_enabled(void);
> >
> > diff --git a/fs/fuse/iomap_i.h b/fs/fuse/iomap_i.h
> > index 6d9ce9c0f40a04..3615ec76c0dec0 100644
> > --- a/fs/fuse/iomap_i.h
> > +++ b/fs/fuse/iomap_i.h
> > @@ -6,19 +6,29 @@
> >  #ifndef _FS_FUSE_IOMAP_I_H
> >  #define _FS_FUSE_IOMAP_I_H
> >
> > +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG_DEFAULT)
> > +DECLARE_STATIC_KEY_TRUE(fuse_iomap_debug);
> > +#else
> > +DECLARE_STATIC_KEY_FALSE(fuse_iomap_debug);
> > +#endif
> > +
> >  #if IS_ENABLED(CONFIG_FUSE_IOMAP)
> >  #if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> > -# define ASSERT(condition) do {                                                \
> > +# define ASSERT(condition) \
> > +while (static_branch_unlikely(&fuse_iomap_debug)) {                    \
> >         int __cond = !!(condition);                                     \
> >         if (unlikely(!__cond))                                          \
> >                 trace_fuse_iomap_assert(__func__, __LINE__, #condition); \
> >         WARN(!__cond, "Assertion failed: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> > -} while (0)
> > +       break;                                                          \
> > +}
> >  # define BAD_DATA(condition) ({                                                \
> >         int __cond = !!(condition);                                     \
> >         if (unlikely(__cond))                                           \
> >                 trace_fuse_iomap_bad_data(__func__, __LINE__, #condition); \
> > -       WARN(__cond, "Bad mapping: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> > +       if (static_branch_unlikely(&fuse_iomap_debug))                  \
> > +               WARN(__cond, "Bad mapping: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> > +       unlikely(__cond);                                                               \
> >  })
> >  #else
> >  # define ASSERT(condition)
> > diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> > index 934d48076a010c..bb867afe6e867c 100644
> > --- a/fs/fuse/Kconfig
> > +++ b/fs/fuse/Kconfig
> > @@ -101,6 +101,21 @@ config FUSE_IOMAP_DEBUG
> >           Enable debugging assertions for the fuse iomap code paths and logging
> >           of bad iomap file mapping data being sent to the kernel.
> >
> > +         Say N here if you don't want any debugging code code compiled in at
> > +         all.
> > +
> > +config FUSE_IOMAP_DEBUG_BY_DEFAULT
> > +       bool "Debug FUSE file IO over iomap at boot time"
> > +       default n
> > +       depends on FUSE_IOMAP_DEBUG
> > +       help
> > +         At boot time, enable debugging assertions for the fuse iomap code
> > +         paths and warnings about bad iomap file mapping data.  This enables
> > +         fuse server authors to control debugging at runtime even on a
> > +         distribution kernel while avoiding most of the overhead on production
> > +         systems.  The setting can be changed at runtime via
> > +         /sys/fs/fuse/iomap/debug.
> > +
> >  config FUSE_IO_URING
> >         bool "FUSE communication over io-uring"
> >         default y
> > diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> > index a88f5d8d2bce15..b6fc70068c5542 100644
> > --- a/fs/fuse/file_iomap.c
> > +++ b/fs/fuse/file_iomap.c
> > @@ -8,6 +8,12 @@
> >  #include "fuse_trace.h"
> >  #include "iomap_i.h"
> >
> > +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG_DEFAULT)
> > +DEFINE_STATIC_KEY_TRUE(fuse_iomap_debug);
> > +#else
> > +DEFINE_STATIC_KEY_FALSE(fuse_iomap_debug);
> > +#endif
> > +
> >  static bool __read_mostly enable_iomap =
> >  #if IS_ENABLED(CONFIG_FUSE_IOMAP_BY_DEFAULT)
> >         true;
> > @@ -17,6 +23,81 @@ static bool __read_mostly enable_iomap =
> >  module_param(enable_iomap, bool, 0644);
> >  MODULE_PARM_DESC(enable_iomap, "Enable file I/O through iomap");
> >
> > +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> > +static struct kobject *iomap_kobj;
> > +
> > +static ssize_t fuse_iomap_debug_show(struct kobject *kobject,
> > +                                    struct kobj_attribute *a, char *buf)
> > +{
> > +       return sysfs_emit(buf, "%d\n", !!static_key_enabled(&fuse_iomap_debug));
> > +}
> > +
> > +static ssize_t fuse_iomap_debug_store(struct kobject *kobject,
> > +                                     struct kobj_attribute *a,
> > +                                     const char *buf, size_t count)
> > +{
> > +       int ret;
> > +       int val;
> > +
> > +       ret = kstrtoint(buf, 0, &val);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (val < 0 || val > 1)
> > +               return -EINVAL;
> > +
> > +       if (val)
> > +               static_branch_enable(&fuse_iomap_debug);
> > +       else
> > +               static_branch_disable(&fuse_iomap_debug);
> > +
> > +       return count;
> > +}
> > +
> > +#define __INIT_KOBJ_ATTR(_name, _mode, _show, _store)                  \
> > +{                                                                      \
> > +       .attr   = { .name = __stringify(_name), .mode = _mode },        \
> > +       .show   = _show,                                                \
> > +       .store  = _store,                                               \
> > +}
> > +
> > +#define FUSE_ATTR_RW(_name, _show, _store)                     \
> > +       static struct kobj_attribute fuse_attr_##_name =        \
> > +                       __INIT_KOBJ_ATTR(_name, 0644, _show, _store)
> > +
> > +#define FUSE_ATTR_PTR(_name)                                   \
> > +       (&fuse_attr_##_name.attr)
> > +
> > +FUSE_ATTR_RW(debug, fuse_iomap_debug_show, fuse_iomap_debug_store);
> > +
> > +static const struct attribute *fuse_iomap_attrs[] = {
> > +       FUSE_ATTR_PTR(debug),
> > +       NULL,
> > +};
> > +
> > +int fuse_iomap_sysfs_init(struct kobject *fuse_kobj)
> > +{
> > +       int error;
> > +
> > +       iomap_kobj = kobject_create_and_add("iomap", fuse_kobj);
> > +       if (!iomap_kobj)
> > +               return -ENOMEM;
> > +
> > +       error = sysfs_create_files(iomap_kobj, fuse_iomap_attrs);
> > +       if (error) {
> > +               kobject_put(iomap_kobj);
> > +               return error;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +void fuse_iomap_sysfs_cleanup(struct kobject *fuse_kobj)
> > +{
> 
> Is sysfs_remove_files() also needed here?

kobject_put is supposed to tear down the attrs that sysfs_create_files
attaches to iomap_kobj.  Though you're right to be suspicious -- there
are a lot of places that explicitly call sysfs_remove_files to undo
sysfs_create_files; and also a lot of places that just let kobject_put
do the dirty work.

> > +       kobject_put(iomap_kobj);
> > +}
> > +#endif /* IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG) */
> > +
> >  bool fuse_iomap_enabled(void)
> >  {
> >         /* Don't let anyone touch iomap until the end of the patchset. */
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 1eea8dc6e723c6..eec711302a4a13 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -2277,8 +2277,14 @@ static int fuse_sysfs_init(void)
> >         if (err)
> >                 goto out_fuse_unregister;
> >
> > +       err = fuse_iomap_sysfs_init(fuse_kobj);
> > +       if (err)
> > +               goto out_fuse_connections;
> > +
> >         return 0;
> >
> > + out_fuse_connections:
> > +       sysfs_remove_mount_point(fuse_kobj, "connections");
> >   out_fuse_unregister:
> >         kobject_put(fuse_kobj);
> >   out_err:
> > @@ -2287,6 +2293,7 @@ static int fuse_sysfs_init(void)
> >
> >  static void fuse_sysfs_cleanup(void)
> >  {
> > +       fuse_iomap_sysfs_cleanup(fuse_kobj);
> >         sysfs_remove_mount_point(fuse_kobj, "connections");
> >         kobject_put(fuse_kobj);
> >  }
> >
> Could you explain why it's better that this goes through sysfs than
> through a module param?

You can dynamically enable debugging on a production system.  I (by
which I really mean the support org) wishes they could do that with XFS.

Module parameters don't come with setter functions so you can't call
static_branch_{enable,disable} when the parameter value updates.

--D

> Thanks,
> Joanne