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@xxxxxxxxxx>
>> ---
>>
>> 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)
>>
