| Message ID | 97e16da4-e5ad-3049-0f6b-c1e24462e035@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: show build options in sysfs | expand |
On Thu, Jun 06, 2019 at 10:30:09PM -0500, Eric Sandeen wrote: > This adds the "build options" string to a sysfs entry: > > # cat /sys/fs/xfs/features/build_opts > ACLs, security attributes, realtime, scrub, no debug > > because right now we only get it in dmesg and scraping dmesg > is not a great option. > > This is really /build options/, not features as in "on-disk, > superblock features" like XFS_SB_FEAT_* - putting this under a > features/ dir will leave the window open open to do supported > superblock features ala ext4 & f2fs in the future if desired. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > No sure if it would make sense to have i.e. > > /sys/fs/xfs/features/build_options > /sys/fs/xfs/features/finobt > /sys/fs/xfs/features/rmapbt > ... > > all in the same features/ dir ? > What's the purpose of the features dir, and why an entry per feature as opposed to a single 'features' file like we're adding for build_opts? Would those per-feature files present any data? The patch seems reasonable to me in general, but I'd prefer to see the directory structure thing at least hashed out before we decide on this kind of placement (as opposed to something like /sys/fs/xfs/build_opts, if that is possible). I see that ext4 has a per-file feature dir along these lines: $ ls /sys/fs/ext4/features/ batched_discard encryption lazy_itable_init meta_bg_resize metadata_csum_seed $ cat /sys/fs/ext4/features/* supported supported supported supported supported I'm not sure if those files disappear when a feature is not available or persist and return something other than "supported?" Are those files used by anything in userspace? > Also I didn't test module unload/teardown as I'm testing on xfs root. > insmod/rmmod works on a quick test on one of my VMs. Brian > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index a14d11d78bd8..bc0e7fd63567 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -55,9 +55,10 @@ > static const struct super_operations xfs_super_operations; > struct bio_set xfs_ioend_bioset; > > -static struct kset *xfs_kset; /* top-level xfs sysfs dir */ > +static struct kset *xfs_kset; /* top-level xfs sysfs dir */ > +static struct xfs_kobj xfs_features_kobj; /* global features */ > #ifdef DEBUG > -static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ > +static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ > #endif > > /* > @@ -2134,11 +2135,16 @@ init_xfs_fs(void) > if (error) > goto out_free_stats; > > + xfs_features_kobj.kobject.kset = xfs_kset; > + error = xfs_sysfs_init(&xfs_features_kobj, &xfs_features_ktype, > + NULL, "features"); > + if (error) > + goto out_remove_stats_kobj; > #ifdef DEBUG > xfs_dbg_kobj.kobject.kset = xfs_kset; > error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug"); > if (error) > - goto out_remove_stats_kobj; > + goto out_remove_features_kobj; > #endif > > error = xfs_qm_init(); > @@ -2155,8 +2161,10 @@ init_xfs_fs(void) > out_remove_dbg_kobj: > #ifdef DEBUG > xfs_sysfs_del(&xfs_dbg_kobj); > - out_remove_stats_kobj: > + out_remove_features_kobj: > #endif > + xfs_sysfs_del(&xfs_features_kobj); > + out_remove_stats_kobj: > xfs_sysfs_del(&xfsstats.xs_kobj); > out_free_stats: > free_percpu(xfsstats.xs_stats); > @@ -2186,6 +2194,7 @@ exit_xfs_fs(void) > #ifdef DEBUG > xfs_sysfs_del(&xfs_dbg_kobj); > #endif > + xfs_sysfs_del(&xfs_features_kobj); > xfs_sysfs_del(&xfsstats.xs_kobj); > free_percpu(xfsstats.xs_stats); > kset_unregister(xfs_kset); > diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c > index cabda13f3c64..98f36ad16237 100644 > --- a/fs/xfs/xfs_sysfs.c > +++ b/fs/xfs/xfs_sysfs.c > @@ -222,6 +222,28 @@ struct kobj_type xfs_dbg_ktype = { > > #endif /* DEBUG */ > > +/* features */ > + > +STATIC ssize_t > +build_opts_show( > + struct kobject *kobject, > + char *buf) > +{ > + return snprintf(buf, PAGE_SIZE, "%s\n", XFS_BUILD_OPTIONS); > +} > +XFS_SYSFS_ATTR_RO(build_opts); > + > +static struct attribute *xfs_features_attrs[] = { > + ATTR_LIST(build_opts), > + NULL, > +}; > + > +struct kobj_type xfs_features_ktype = { > + .release = xfs_sysfs_release, > + .sysfs_ops = &xfs_sysfs_ops, > + .default_attrs = xfs_features_attrs, > +}; > + > /* stats */ > > static inline struct xstats * > diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h > index e9f810fc6731..e475f6b7eb91 100644 > --- a/fs/xfs/xfs_sysfs.h > +++ b/fs/xfs/xfs_sysfs.h > @@ -11,6 +11,7 @@ extern struct kobj_type xfs_mp_ktype; /* xfs_mount */ > extern struct kobj_type xfs_dbg_ktype; /* debug */ > extern struct kobj_type xfs_log_ktype; /* xlog */ > extern struct kobj_type xfs_stats_ktype; /* stats */ > +extern struct kobj_type xfs_features_ktype; /* features*/ > > static inline struct xfs_kobj * > to_kobj(struct kobject *kobject) >
On 6/7/19 8:20 AM, Brian Foster wrote: > On Thu, Jun 06, 2019 at 10:30:09PM -0500, Eric Sandeen wrote: >> This adds the "build options" string to a sysfs entry: >> >> # cat /sys/fs/xfs/features/build_opts >> ACLs, security attributes, realtime, scrub, no debug >> >> because right now we only get it in dmesg and scraping dmesg >> is not a great option. >> >> This is really /build options/, not features as in "on-disk, >> superblock features" like XFS_SB_FEAT_* - putting this under a >> features/ dir will leave the window open open to do supported >> superblock features ala ext4 & f2fs in the future if desired. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> No sure if it would make sense to have i.e. >> >> /sys/fs/xfs/features/build_options >> /sys/fs/xfs/features/finobt >> /sys/fs/xfs/features/rmapbt >> ... >> >> all in the same features/ dir ? >> > > What's the purpose of the features dir, and why an entry per feature as > opposed to a single 'features' file like we're adding for build_opts? just because ext4 and f2fs did it that way, and supposedly sysfs is one value per file. also our entire sysfs structure is based around having dirs under xfs/ - tbh I haven't yet sorted out how to actually ad a bare file under xfs/ ;) Of course it's possible but existing infra in our code is friendlier with subdirs of files. ;) > Would those per-feature files present any data? The patch seems > reasonable to me in general, but I'd prefer to see the directory > structure thing at least hashed out before we decide on this kind of > placement (as opposed to something like /sys/fs/xfs/build_opts, if that > is possible). Yeah, that's why I raised the question above, stupid me ;) > I see that ext4 has a per-file feature dir along these lines: > > $ ls /sys/fs/ext4/features/ > batched_discard encryption lazy_itable_init meta_bg_resize metadata_csum_seed > $ cat /sys/fs/ext4/features/* > supported > supported > supported > supported > supported > > I'm not sure if those files disappear when a feature is not available or > persist and return something other than "supported?" Are those files > used by anything in userspace? It's based on what the running code can support, i.e. the total of its COMPAT_FEATURES stuff. >> Also I didn't test module unload/teardown as I'm testing on xfs root. >> > > insmod/rmmod works on a quick test on one of my VMs. thanks, sorry for being lazy there. -Eric > Brian > >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c >> index a14d11d78bd8..bc0e7fd63567 100644 >> --- a/fs/xfs/xfs_super.c >> +++ b/fs/xfs/xfs_super.c >> @@ -55,9 +55,10 @@ >> static const struct super_operations xfs_super_operations; >> struct bio_set xfs_ioend_bioset; >> >> -static struct kset *xfs_kset; /* top-level xfs sysfs dir */ >> +static struct kset *xfs_kset; /* top-level xfs sysfs dir */ >> +static struct xfs_kobj xfs_features_kobj; /* global features */ >> #ifdef DEBUG >> -static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ >> +static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ >> #endif >> >> /* >> @@ -2134,11 +2135,16 @@ init_xfs_fs(void) >> if (error) >> goto out_free_stats; >> >> + xfs_features_kobj.kobject.kset = xfs_kset; >> + error = xfs_sysfs_init(&xfs_features_kobj, &xfs_features_ktype, >> + NULL, "features"); >> + if (error) >> + goto out_remove_stats_kobj; >> #ifdef DEBUG >> xfs_dbg_kobj.kobject.kset = xfs_kset; >> error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug"); >> if (error) >> - goto out_remove_stats_kobj; >> + goto out_remove_features_kobj; >> #endif >> >> error = xfs_qm_init(); >> @@ -2155,8 +2161,10 @@ init_xfs_fs(void) >> out_remove_dbg_kobj: >> #ifdef DEBUG >> xfs_sysfs_del(&xfs_dbg_kobj); >> - out_remove_stats_kobj: >> + out_remove_features_kobj: >> #endif >> + xfs_sysfs_del(&xfs_features_kobj); >> + out_remove_stats_kobj: >> xfs_sysfs_del(&xfsstats.xs_kobj); >> out_free_stats: >> free_percpu(xfsstats.xs_stats); >> @@ -2186,6 +2194,7 @@ exit_xfs_fs(void) >> #ifdef DEBUG >> xfs_sysfs_del(&xfs_dbg_kobj); >> #endif >> + xfs_sysfs_del(&xfs_features_kobj); >> xfs_sysfs_del(&xfsstats.xs_kobj); >> free_percpu(xfsstats.xs_stats); >> kset_unregister(xfs_kset); >> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c >> index cabda13f3c64..98f36ad16237 100644 >> --- a/fs/xfs/xfs_sysfs.c >> +++ b/fs/xfs/xfs_sysfs.c >> @@ -222,6 +222,28 @@ struct kobj_type xfs_dbg_ktype = { >> >> #endif /* DEBUG */ >> >> +/* features */ >> + >> +STATIC ssize_t >> +build_opts_show( >> + struct kobject *kobject, >> + char *buf) >> +{ >> + return snprintf(buf, PAGE_SIZE, "%s\n", XFS_BUILD_OPTIONS); >> +} >> +XFS_SYSFS_ATTR_RO(build_opts); >> + >> +static struct attribute *xfs_features_attrs[] = { >> + ATTR_LIST(build_opts), >> + NULL, >> +}; >> + >> +struct kobj_type xfs_features_ktype = { >> + .release = xfs_sysfs_release, >> + .sysfs_ops = &xfs_sysfs_ops, >> + .default_attrs = xfs_features_attrs, >> +}; >> + >> /* stats */ >> >> static inline struct xstats * >> diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h >> index e9f810fc6731..e475f6b7eb91 100644 >> --- a/fs/xfs/xfs_sysfs.h >> +++ b/fs/xfs/xfs_sysfs.h >> @@ -11,6 +11,7 @@ extern struct kobj_type xfs_mp_ktype; /* xfs_mount */ >> extern struct kobj_type xfs_dbg_ktype; /* debug */ >> extern struct kobj_type xfs_log_ktype; /* xlog */ >> extern struct kobj_type xfs_stats_ktype; /* stats */ >> +extern struct kobj_type xfs_features_ktype; /* features*/ >> >> static inline struct xfs_kobj * >> to_kobj(struct kobject *kobject) >>
On Fri, Jun 07, 2019 at 08:41:09AM -0500, Eric Sandeen wrote: > On 6/7/19 8:20 AM, Brian Foster wrote: > > On Thu, Jun 06, 2019 at 10:30:09PM -0500, Eric Sandeen wrote: > >> This adds the "build options" string to a sysfs entry: > >> > >> # cat /sys/fs/xfs/features/build_opts > >> ACLs, security attributes, realtime, scrub, no debug > >> > >> because right now we only get it in dmesg and scraping dmesg > >> is not a great option. > >> > >> This is really /build options/, not features as in "on-disk, > >> superblock features" like XFS_SB_FEAT_* - putting this under a > >> features/ dir will leave the window open open to do supported > >> superblock features ala ext4 & f2fs in the future if desired. > >> > >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> > >> --- > >> > >> No sure if it would make sense to have i.e. > >> > >> /sys/fs/xfs/features/build_options > >> /sys/fs/xfs/features/finobt > >> /sys/fs/xfs/features/rmapbt > >> ... > >> > >> all in the same features/ dir ? > >> > > > > What's the purpose of the features dir, and why an entry per feature as > > opposed to a single 'features' file like we're adding for build_opts? > > just because ext4 and f2fs did it that way, and supposedly sysfs > is one value per file. > > also our entire sysfs structure is based around having dirs under xfs/ > - tbh I haven't yet sorted out how to actually ad a bare file under > xfs/ ;) Of course it's possible but existing infra in our code is friendlier > with subdirs of files. ;) sysfs_create_file(&xfs_kset.kobj, ATTR_LIST(build_opts)); ? sysfs is such a pain... I'm pretty sure that's a gross hack since a kset doesn't look like it's supposed to have attributes. > > > Would those per-feature files present any data? The patch seems > > reasonable to me in general, but I'd prefer to see the directory > > structure thing at least hashed out before we decide on this kind of > > placement (as opposed to something like /sys/fs/xfs/build_opts, if that > > is possible). > > Yeah, that's why I raised the question above, stupid me ;) > > > I see that ext4 has a per-file feature dir along these lines: > > > > $ ls /sys/fs/ext4/features/ > > batched_discard encryption lazy_itable_init meta_bg_resize metadata_csum_seed > > $ cat /sys/fs/ext4/features/* > > supported > > supported > > supported > > supported > > supported > > > > I'm not sure if those files disappear when a feature is not available or > > persist and return something other than "supported?" Are those files > > used by anything in userspace? > > It's based on what the running code can support, i.e. the total of its > COMPAT_FEATURES stuff. If you do, can we please build a feature flags to strings decoder ring so that xfs_db/kernel/mkfs/repair can standardize the names for all the features? I'd like to avoid the spinodes/sparse confusion again. --D > > >> Also I didn't test module unload/teardown as I'm testing on xfs root. > >> > > > > insmod/rmmod works on a quick test on one of my VMs. > > thanks, sorry for being lazy there. > > -Eric > > > Brian > > > >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > >> index a14d11d78bd8..bc0e7fd63567 100644 > >> --- a/fs/xfs/xfs_super.c > >> +++ b/fs/xfs/xfs_super.c > >> @@ -55,9 +55,10 @@ > >> static const struct super_operations xfs_super_operations; > >> struct bio_set xfs_ioend_bioset; > >> > >> -static struct kset *xfs_kset; /* top-level xfs sysfs dir */ > >> +static struct kset *xfs_kset; /* top-level xfs sysfs dir */ > >> +static struct xfs_kobj xfs_features_kobj; /* global features */ > >> #ifdef DEBUG > >> -static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ > >> +static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ > >> #endif > >> > >> /* > >> @@ -2134,11 +2135,16 @@ init_xfs_fs(void) > >> if (error) > >> goto out_free_stats; > >> > >> + xfs_features_kobj.kobject.kset = xfs_kset; > >> + error = xfs_sysfs_init(&xfs_features_kobj, &xfs_features_ktype, > >> + NULL, "features"); > >> + if (error) > >> + goto out_remove_stats_kobj; > >> #ifdef DEBUG > >> xfs_dbg_kobj.kobject.kset = xfs_kset; > >> error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug"); > >> if (error) > >> - goto out_remove_stats_kobj; > >> + goto out_remove_features_kobj; > >> #endif > >> > >> error = xfs_qm_init(); > >> @@ -2155,8 +2161,10 @@ init_xfs_fs(void) > >> out_remove_dbg_kobj: > >> #ifdef DEBUG > >> xfs_sysfs_del(&xfs_dbg_kobj); > >> - out_remove_stats_kobj: > >> + out_remove_features_kobj: > >> #endif > >> + xfs_sysfs_del(&xfs_features_kobj); > >> + out_remove_stats_kobj: > >> xfs_sysfs_del(&xfsstats.xs_kobj); > >> out_free_stats: > >> free_percpu(xfsstats.xs_stats); > >> @@ -2186,6 +2194,7 @@ exit_xfs_fs(void) > >> #ifdef DEBUG > >> xfs_sysfs_del(&xfs_dbg_kobj); > >> #endif > >> + xfs_sysfs_del(&xfs_features_kobj); > >> xfs_sysfs_del(&xfsstats.xs_kobj); > >> free_percpu(xfsstats.xs_stats); > >> kset_unregister(xfs_kset); > >> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c > >> index cabda13f3c64..98f36ad16237 100644 > >> --- a/fs/xfs/xfs_sysfs.c > >> +++ b/fs/xfs/xfs_sysfs.c > >> @@ -222,6 +222,28 @@ struct kobj_type xfs_dbg_ktype = { > >> > >> #endif /* DEBUG */ > >> > >> +/* features */ > >> + > >> +STATIC ssize_t > >> +build_opts_show( > >> + struct kobject *kobject, > >> + char *buf) > >> +{ > >> + return snprintf(buf, PAGE_SIZE, "%s\n", XFS_BUILD_OPTIONS); > >> +} > >> +XFS_SYSFS_ATTR_RO(build_opts); > >> + > >> +static struct attribute *xfs_features_attrs[] = { > >> + ATTR_LIST(build_opts), > >> + NULL, > >> +}; > >> + > >> +struct kobj_type xfs_features_ktype = { > >> + .release = xfs_sysfs_release, > >> + .sysfs_ops = &xfs_sysfs_ops, > >> + .default_attrs = xfs_features_attrs, > >> +}; > >> + > >> /* stats */ > >> > >> static inline struct xstats * > >> diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h > >> index e9f810fc6731..e475f6b7eb91 100644 > >> --- a/fs/xfs/xfs_sysfs.h > >> +++ b/fs/xfs/xfs_sysfs.h > >> @@ -11,6 +11,7 @@ extern struct kobj_type xfs_mp_ktype; /* xfs_mount */ > >> extern struct kobj_type xfs_dbg_ktype; /* debug */ > >> extern struct kobj_type xfs_log_ktype; /* xlog */ > >> extern struct kobj_type xfs_stats_ktype; /* stats */ > >> +extern struct kobj_type xfs_features_ktype; /* features*/ > >> > >> static inline struct xfs_kobj * > >> to_kobj(struct kobject *kobject) > >> >
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index a14d11d78bd8..bc0e7fd63567 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -55,9 +55,10 @@ static const struct super_operations xfs_super_operations; struct bio_set xfs_ioend_bioset; -static struct kset *xfs_kset; /* top-level xfs sysfs dir */ +static struct kset *xfs_kset; /* top-level xfs sysfs dir */ +static struct xfs_kobj xfs_features_kobj; /* global features */ #ifdef DEBUG -static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ +static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ #endif /* @@ -2134,11 +2135,16 @@ init_xfs_fs(void) if (error) goto out_free_stats; + xfs_features_kobj.kobject.kset = xfs_kset; + error = xfs_sysfs_init(&xfs_features_kobj, &xfs_features_ktype, + NULL, "features"); + if (error) + goto out_remove_stats_kobj; #ifdef DEBUG xfs_dbg_kobj.kobject.kset = xfs_kset; error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug"); if (error) - goto out_remove_stats_kobj; + goto out_remove_features_kobj; #endif error = xfs_qm_init(); @@ -2155,8 +2161,10 @@ init_xfs_fs(void) out_remove_dbg_kobj: #ifdef DEBUG xfs_sysfs_del(&xfs_dbg_kobj); - out_remove_stats_kobj: + out_remove_features_kobj: #endif + xfs_sysfs_del(&xfs_features_kobj); + out_remove_stats_kobj: xfs_sysfs_del(&xfsstats.xs_kobj); out_free_stats: free_percpu(xfsstats.xs_stats); @@ -2186,6 +2194,7 @@ exit_xfs_fs(void) #ifdef DEBUG xfs_sysfs_del(&xfs_dbg_kobj); #endif + xfs_sysfs_del(&xfs_features_kobj); xfs_sysfs_del(&xfsstats.xs_kobj); free_percpu(xfsstats.xs_stats); kset_unregister(xfs_kset); diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c index cabda13f3c64..98f36ad16237 100644 --- a/fs/xfs/xfs_sysfs.c +++ b/fs/xfs/xfs_sysfs.c @@ -222,6 +222,28 @@ struct kobj_type xfs_dbg_ktype = { #endif /* DEBUG */ +/* features */ + +STATIC ssize_t +build_opts_show( + struct kobject *kobject, + char *buf) +{ + return snprintf(buf, PAGE_SIZE, "%s\n", XFS_BUILD_OPTIONS); +} +XFS_SYSFS_ATTR_RO(build_opts); + +static struct attribute *xfs_features_attrs[] = { + ATTR_LIST(build_opts), + NULL, +}; + +struct kobj_type xfs_features_ktype = { + .release = xfs_sysfs_release, + .sysfs_ops = &xfs_sysfs_ops, + .default_attrs = xfs_features_attrs, +}; + /* stats */ static inline struct xstats * diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h index e9f810fc6731..e475f6b7eb91 100644 --- a/fs/xfs/xfs_sysfs.h +++ b/fs/xfs/xfs_sysfs.h @@ -11,6 +11,7 @@ extern struct kobj_type xfs_mp_ktype; /* xfs_mount */ extern struct kobj_type xfs_dbg_ktype; /* debug */ extern struct kobj_type xfs_log_ktype; /* xlog */ extern struct kobj_type xfs_stats_ktype; /* stats */ +extern struct kobj_type xfs_features_ktype; /* features*/ static inline struct xfs_kobj * to_kobj(struct kobject *kobject)
This adds the "build options" string to a sysfs entry: # cat /sys/fs/xfs/features/build_opts ACLs, security attributes, realtime, scrub, no debug because right now we only get it in dmesg and scraping dmesg is not a great option. This is really /build options/, not features as in "on-disk, superblock features" like XFS_SB_FEAT_* - putting this under a features/ dir will leave the window open open to do supported superblock features ala ext4 & f2fs in the future if desired. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- No sure if it would make sense to have i.e. /sys/fs/xfs/features/build_options /sys/fs/xfs/features/finobt /sys/fs/xfs/features/rmapbt ... all in the same features/ dir ? Also I didn't test module unload/teardown as I'm testing on xfs root.