Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 18 ++-
fs/f2fs/f2fs.h | 3 +
fs/f2fs/sysfs.c | 152 +++++++++++++++++++++++-
3 files changed, 168 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 5088281e312e..43b2cde80b70 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -203,7 +203,23 @@ Description: Shows total written kbytes issued to disk.
What: /sys/fs/f2fs/<disk>/features
Date: July 2017
Contact: "Jaegeuk Kim" <[email protected]>
-Description: Shows all enabled features in current device.
+Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
+ Shows all enabled features in current device.
+ Supported features:
+ encryption, blkzoned, extra_attr, projquota, inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression,
+ encrypted_casefold, pin_file.
+
+What: /sys/fs/f2fs/<disk>/feature_list/
+Date: June 2021
+Contact: "Jaegeuk Kim" <[email protected]>
+Description: Expand /sys/fs/f2fs/<disk>/features to meet sysfs rule.
+ Supported features:
+ encryption, block_zoned, extra_attr, projquota, inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression,
+ encrypted_casefold, pin_file.
What: /sys/fs/f2fs/<disk>/inject_rate
Date: May 2016
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8903c43091f8..17ade71547f1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1665,6 +1665,9 @@ struct f2fs_sb_info {
struct kobject s_stat_kobj; /* /sys/fs/f2fs/<devname>/stat */
struct completion s_stat_kobj_unregister;
+ struct kobject s_disk_feat_kobj; /* /sys/fs/f2fs/<devname>/feature_list */
+ struct completion s_disk_feat_kobj_unregister;
+
/* For shrinker support */
struct list_head s_list;
int s_ndevs; /* number of devices */
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 2a76c959a7b4..5d591f0b79b7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -587,32 +587,85 @@ enum feat_id {
FEAT_RO,
FEAT_TEST_DUMMY_ENCRYPTION_V2,
FEAT_ENCRYPTED_CASEFOLD,
+ FEAT_PIN_FILE,
};
static ssize_t f2fs_feature_show(struct f2fs_attr *a,
struct f2fs_sb_info *sbi, char *buf)
{
+ unsigned long feat_supp = 0;
+
switch (a->id) {
case FEAT_CRYPTO:
+ feat_supp |= f2fs_sb_has_encrypt(sbi) ?
+ (1 << FEAT_CRYPTO) : 0;
+ fallthrough;
case FEAT_BLKZONED:
- case FEAT_ATOMIC_WRITE:
+ feat_supp |= f2fs_sb_has_blkzoned(sbi) ?
+ (1 << FEAT_BLKZONED) : 0;
+ fallthrough;
case FEAT_EXTRA_ATTR:
+ feat_supp |= f2fs_sb_has_extra_attr(sbi) ?
+ (1 << FEAT_EXTRA_ATTR) : 0;
+ fallthrough;
case FEAT_PROJECT_QUOTA:
+ feat_supp |= f2fs_sb_has_project_quota(sbi) ?
+ (1 << FEAT_PROJECT_QUOTA) : 0;
+ fallthrough;
case FEAT_INODE_CHECKSUM:
+ feat_supp |= f2fs_sb_has_inode_chksum(sbi) ?
+ (1 << FEAT_INODE_CHECKSUM) : 0;
+ fallthrough;
case FEAT_FLEXIBLE_INLINE_XATTR:
+ feat_supp |= f2fs_sb_has_flexible_inline_xattr(sbi) ?
+ (1 << FEAT_FLEXIBLE_INLINE_XATTR) : 0;
+ fallthrough;
case FEAT_QUOTA_INO:
+ feat_supp |= f2fs_sb_has_quota_ino(sbi) ?
+ (1 << FEAT_QUOTA_INO) : 0;
+ fallthrough;
case FEAT_INODE_CRTIME:
+ feat_supp |= f2fs_sb_has_inode_crtime(sbi) ?
+ (1 << FEAT_INODE_CRTIME) : 0;
+ fallthrough;
case FEAT_LOST_FOUND:
+ feat_supp |= f2fs_sb_has_lost_found(sbi) ?
+ (1 << FEAT_LOST_FOUND) : 0;
+ fallthrough;
case FEAT_VERITY:
+ feat_supp |= f2fs_sb_has_verity(sbi) ?
+ (1 << FEAT_VERITY) : 0;
+ fallthrough;
case FEAT_SB_CHECKSUM:
+ feat_supp |= f2fs_sb_has_sb_chksum(sbi) ?
+ (1 << FEAT_SB_CHECKSUM) : 0;
+ fallthrough;
case FEAT_CASEFOLD:
+ feat_supp |= f2fs_sb_has_casefold(sbi) ?
+ (1 << FEAT_CASEFOLD) : 0;
+ fallthrough;
case FEAT_COMPRESSION:
+ feat_supp |= f2fs_sb_has_compression(sbi) ?
+ (1 << FEAT_COMPRESSION) : 0;
+ fallthrough;
case FEAT_RO:
- case FEAT_TEST_DUMMY_ENCRYPTION_V2:
+ feat_supp |= f2fs_sb_has_readonly(sbi) ?
+ (1 << FEAT_RO) : 0;
+ fallthrough;
case FEAT_ENCRYPTED_CASEFOLD:
- return sprintf(buf, "supported\n");
+ feat_supp |= (f2fs_sb_has_casefold(sbi) &&
+ f2fs_sb_has_encrypt(sbi)) ?
+ (1 << FEAT_ENCRYPTED_CASEFOLD) : 0;
+ fallthrough;
+ case FEAT_PIN_FILE:
+ feat_supp |= (1 << FEAT_PIN_FILE);
+ fallthrough;
+ case FEAT_TEST_DUMMY_ENCRYPTION_V2:
+ case FEAT_ATOMIC_WRITE:
+ if (!a->offset || feat_supp & (1 << a->id))
+ return sprintf(buf, "supported\n");
}
- return 0;
+ return sprintf(buf, "not supported\n");
}
#define F2FS_ATTR_OFFSET(_struct_type, _name, _mode, _show, _store, _offset) \
@@ -636,6 +689,7 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
static struct f2fs_attr f2fs_attr_##_name = { \
.attr = {.name = __stringify(_name), .mode = 0444 }, \
.show = f2fs_feature_show, \
+ .offset = 0, \
.id = _id, \
}
@@ -743,6 +797,7 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
#endif
+F2FS_FEATURE_RO_ATTR(pin_file, FEAT_PIN_FILE);
/* For ATGC */
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_ratio, candidate_ratio);
@@ -856,6 +911,7 @@ static struct attribute *f2fs_feat_attrs[] = {
#ifdef CONFIG_F2FS_FS_COMPRESSION
ATTR_LIST(compression),
#endif
+ ATTR_LIST(pin_file),
NULL,
};
ATTRIBUTE_GROUPS(f2fs_feat);
@@ -867,6 +923,52 @@ static struct attribute *f2fs_stat_attrs[] = {
};
ATTRIBUTE_GROUPS(f2fs_stat);
+#define F2FS_DISK_FEATURE_RO_ATTR(_name, _id) \
+static struct f2fs_attr f2fs_attr_disk_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_feature_show, \
+ .offset = 1, \
+ .id = _id, \
+}
+
+F2FS_DISK_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
+F2FS_DISK_FEATURE_RO_ATTR(block_zoned, FEAT_BLKZONED);
+F2FS_DISK_FEATURE_RO_ATTR(extra_attr, FEAT_EXTRA_ATTR);
+F2FS_DISK_FEATURE_RO_ATTR(project_quota, FEAT_PROJECT_QUOTA);
+F2FS_DISK_FEATURE_RO_ATTR(inode_checksum, FEAT_INODE_CHECKSUM);
+F2FS_DISK_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
+F2FS_DISK_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
+F2FS_DISK_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
+F2FS_DISK_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
+F2FS_DISK_FEATURE_RO_ATTR(verity, FEAT_VERITY);
+F2FS_DISK_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
+F2FS_DISK_FEATURE_RO_ATTR(casefold, FEAT_CASEFOLD);
+F2FS_DISK_FEATURE_RO_ATTR(compression, FEAT_COMPRESSION);
+F2FS_DISK_FEATURE_RO_ATTR(readonly, FEAT_RO);
+F2FS_DISK_FEATURE_RO_ATTR(encrypted_casefold, FEAT_ENCRYPTED_CASEFOLD);
+F2FS_DISK_FEATURE_RO_ATTR(pin_file, FEAT_PIN_FILE);
+
+static struct attribute *f2fs_disk_feat_attrs[] = {
+ ATTR_LIST(disk_encryption),
+ ATTR_LIST(disk_block_zoned),
+ ATTR_LIST(disk_extra_attr),
+ ATTR_LIST(disk_project_quota),
+ ATTR_LIST(disk_inode_checksum),
+ ATTR_LIST(disk_flexible_inline_xattr),
+ ATTR_LIST(disk_quota_ino),
+ ATTR_LIST(disk_inode_crtime),
+ ATTR_LIST(disk_lost_found),
+ ATTR_LIST(disk_verity),
+ ATTR_LIST(disk_sb_checksum),
+ ATTR_LIST(disk_casefold),
+ ATTR_LIST(disk_compression),
+ ATTR_LIST(disk_readonly),
+ ATTR_LIST(disk_encrypted_casefold),
+ ATTR_LIST(disk_pin_file),
+ NULL,
+};
+ATTRIBUTE_GROUPS(f2fs_disk_feat);
+
static const struct sysfs_ops f2fs_attr_ops = {
.show = f2fs_attr_show,
.store = f2fs_attr_store,
@@ -933,6 +1035,34 @@ static struct kobj_type f2fs_stat_ktype = {
.release = f2fs_stat_kobj_release,
};
+static ssize_t f2fs_disk_feat_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_disk_feat_kobj);
+ struct f2fs_attr *a = container_of(attr, struct f2fs_attr, attr);
+
+ return a->show ? a->show(a, sbi, buf) : 0;
+}
+
+static void f2fs_disk_feat_kobj_release(struct kobject *kobj)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_disk_feat_kobj);
+ complete(&sbi->s_disk_feat_kobj_unregister);
+}
+
+static const struct sysfs_ops f2fs_disk_feat_attr_ops = {
+ .show = f2fs_disk_feat_attr_show,
+};
+
+static struct kobj_type f2fs_disk_feat_ktype = {
+ .default_groups = f2fs_disk_feat_groups,
+ .sysfs_ops = &f2fs_disk_feat_attr_ops,
+ .release = f2fs_disk_feat_kobj_release,
+};
+
+
static int __maybe_unused segment_info_seq_show(struct seq_file *seq,
void *offset)
{
@@ -1149,6 +1279,15 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
if (err)
goto put_stat_kobj;
+ sbi->s_disk_feat_kobj.kset = &f2fs_kset;
+ init_completion(&sbi->s_disk_feat_kobj_unregister);
+ err = kobject_init_and_add(&sbi->s_disk_feat_kobj,
+ &f2fs_disk_feat_ktype,
+ &sbi->s_kobj, "feature_list");
+ if (err)
+ goto put_stat_kobj;
+
+
if (f2fs_proc_root)
sbi->s_proc = proc_mkdir(sb->s_id, f2fs_proc_root);
@@ -1166,6 +1305,8 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
put_stat_kobj:
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
+ kobject_put(&sbi->s_disk_feat_kobj);
+ wait_for_completion(&sbi->s_disk_feat_kobj_unregister);
put_sb_kobj:
kobject_put(&sbi->s_kobj);
wait_for_completion(&sbi->s_kobj_unregister);
@@ -1185,6 +1326,9 @@ void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
kobject_del(&sbi->s_stat_kobj);
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
+ kobject_del(&sbi->s_disk_feat_kobj);
+ kobject_put(&sbi->s_disk_feat_kobj);
+ wait_for_completion(&sbi->s_disk_feat_kobj_unregister);
kobject_del(&sbi->s_kobj);
kobject_put(&sbi->s_kobj);
--
2.32.0.rc1.229.g3e70b5a671-goog
On Thu, Jun 03, 2021 at 03:08:34PM -0700, Jaegeuk Kim wrote:
> Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-fs-f2fs | 18 ++-
> fs/f2fs/f2fs.h | 3 +
> fs/f2fs/sysfs.c | 152 +++++++++++++++++++++++-
> 3 files changed, 168 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 5088281e312e..43b2cde80b70 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -203,7 +203,23 @@ Description: Shows total written kbytes issued to disk.
> What: /sys/fs/f2fs/<disk>/features
> Date: July 2017
> Contact: "Jaegeuk Kim" <[email protected]>
> -Description: Shows all enabled features in current device.
> +Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
> + Shows all enabled features in current device.
> + Supported features:
> + encryption, blkzoned, extra_attr, projquota, inode_checksum,
> + flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
> + verity, sb_checksum, casefold, readonly, compression,
> + encrypted_casefold, pin_file.
Isn't pin_file a feature of the implementation, not of a particular filesystem
instance? I.e. something that should go in /sys/fs/f2fs/features/, not here.
Likewise for encrypted_casefold, as it is implied by encryption && casefold.
> +
> +What: /sys/fs/f2fs/<disk>/feature_list/
> +Date: June 2021
> +Contact: "Jaegeuk Kim" <[email protected]>
> +Description: Expand /sys/fs/f2fs/<disk>/features to meet sysfs rule.
> + Supported features:
> + encryption, block_zoned, extra_attr, projquota, inode_checksum,
> + flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
> + verity, sb_checksum, casefold, readonly, compression,
> + encrypted_casefold, pin_file.
Is it intentional that the file has "blkzoned" but the directory has
"blk_zoned"?
Also, your code has another difference -- "project_quota" is used instead of
"projquota". But that's not mentioned above.
And encrypted_casefold and pin_file don't seem appropriate to include here, as
mentioned above.
> @@ -1665,6 +1665,9 @@ struct f2fs_sb_info {
> struct kobject s_stat_kobj; /* /sys/fs/f2fs/<devname>/stat */
> struct completion s_stat_kobj_unregister;
>
> + struct kobject s_disk_feat_kobj; /* /sys/fs/f2fs/<devname>/feature_list */
> + struct completion s_disk_feat_kobj_unregister;
This is for a particular filesystem instance, not a disk per se. (A f2fs
filesystem can use multiple disks.) So having "disk" in the name doesn't make
sense.
Please use more logical names like s_feature_list_kobj,
f2fs_sb_feature_list_ktype, f2fs_sb_feat_attrs, etc.
> static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> struct f2fs_sb_info *sbi, char *buf)
> {
> + unsigned long feat_supp = 0;
> +
> switch (a->id) {
> case FEAT_CRYPTO:
> + feat_supp |= f2fs_sb_has_encrypt(sbi) ?
> + (1 << FEAT_CRYPTO) : 0;
> + fallthrough;
> case FEAT_BLKZONED:
> - case FEAT_ATOMIC_WRITE:
> + feat_supp |= f2fs_sb_has_blkzoned(sbi) ?
> + (1 << FEAT_BLKZONED) : 0;
> + fallthrough;
> case FEAT_EXTRA_ATTR:
> + feat_supp |= f2fs_sb_has_extra_attr(sbi) ?
> + (1 << FEAT_EXTRA_ATTR) : 0;
> + fallthrough;
> case FEAT_PROJECT_QUOTA:
> + feat_supp |= f2fs_sb_has_project_quota(sbi) ?
> + (1 << FEAT_PROJECT_QUOTA) : 0;
> + fallthrough;
> case FEAT_INODE_CHECKSUM:
> + feat_supp |= f2fs_sb_has_inode_chksum(sbi) ?
> + (1 << FEAT_INODE_CHECKSUM) : 0;
> + fallthrough;
> case FEAT_FLEXIBLE_INLINE_XATTR:
> + feat_supp |= f2fs_sb_has_flexible_inline_xattr(sbi) ?
> + (1 << FEAT_FLEXIBLE_INLINE_XATTR) : 0;
> + fallthrough;
> case FEAT_QUOTA_INO:
> + feat_supp |= f2fs_sb_has_quota_ino(sbi) ?
> + (1 << FEAT_QUOTA_INO) : 0;
> + fallthrough;
> case FEAT_INODE_CRTIME:
> + feat_supp |= f2fs_sb_has_inode_crtime(sbi) ?
> + (1 << FEAT_INODE_CRTIME) : 0;
> + fallthrough;
> case FEAT_LOST_FOUND:
> + feat_supp |= f2fs_sb_has_lost_found(sbi) ?
> + (1 << FEAT_LOST_FOUND) : 0;
> + fallthrough;
> case FEAT_VERITY:
> + feat_supp |= f2fs_sb_has_verity(sbi) ?
> + (1 << FEAT_VERITY) : 0;
> + fallthrough;
> case FEAT_SB_CHECKSUM:
> + feat_supp |= f2fs_sb_has_sb_chksum(sbi) ?
> + (1 << FEAT_SB_CHECKSUM) : 0;
> + fallthrough;
> case FEAT_CASEFOLD:
> + feat_supp |= f2fs_sb_has_casefold(sbi) ?
> + (1 << FEAT_CASEFOLD) : 0;
> + fallthrough;
> case FEAT_COMPRESSION:
> + feat_supp |= f2fs_sb_has_compression(sbi) ?
> + (1 << FEAT_COMPRESSION) : 0;
> + fallthrough;
> case FEAT_RO:
> - case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> + feat_supp |= f2fs_sb_has_readonly(sbi) ?
> + (1 << FEAT_RO) : 0;
> + fallthrough;
> case FEAT_ENCRYPTED_CASEFOLD:
> - return sprintf(buf, "supported\n");
> + feat_supp |= (f2fs_sb_has_casefold(sbi) &&
> + f2fs_sb_has_encrypt(sbi)) ?
> + (1 << FEAT_ENCRYPTED_CASEFOLD) : 0;
> + fallthrough;
> + case FEAT_PIN_FILE:
> + feat_supp |= (1 << FEAT_PIN_FILE);
> + fallthrough;
> + case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> + case FEAT_ATOMIC_WRITE:
> + if (!a->offset || feat_supp & (1 << a->id))
> + return sprintf(buf, "supported\n");
> }
> - return 0;
> + return sprintf(buf, "not supported\n");
> }
This function doesn't make much sense.
Part of the problem is that the same function is handling both
/sys/fs/f2fs/features/ and /sys/fs/f2fs/$s_id/feature_list/.
All the former needs is to print "supported", since unsupported is indicated by
the file not being there at all. So it should simply have its own ->show
function separate from the one for feature_list/.
And the feature_list/ ones could just store the F2FS_FEATURE_* bit in
f2fs_attr::id and check for it using F2FS_HAS_FEATURE(). That would be much
simpler -- no need for the feat_id enum or the long switch statement.
Also for feature_list/ it might be better to use "unsupported" than
"not supported", so that \<supported\> doesn't match...
> static struct f2fs_attr f2fs_attr_##_name = { \
> .attr = {.name = __stringify(_name), .mode = 0444 }, \
> .show = f2fs_feature_show, \
> + .offset = 0, \
> .id = _id, \
> }
There's no need to use the .offset argument if features/ and $s_id/feature_list/
just used different ->show functions.
> +#define F2FS_DISK_FEATURE_RO_ATTR(_name, _id) \
F2FS_SB_FEATURE_ATTR would be a much better name, since these pertain to a
filesystem instance (not necessarily a disk), and all the feature attributes are
read-only.
> static int __maybe_unused segment_info_seq_show(struct seq_file *seq,
> void *offset)
> {
> @@ -1149,6 +1279,15 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
> if (err)
> goto put_stat_kobj;
>
> + sbi->s_disk_feat_kobj.kset = &f2fs_kset;
> + init_completion(&sbi->s_disk_feat_kobj_unregister);
> + err = kobject_init_and_add(&sbi->s_disk_feat_kobj,
> + &f2fs_disk_feat_ktype,
> + &sbi->s_kobj, "feature_list");
> + if (err)
> + goto put_stat_kobj;
> +
> +
> if (f2fs_proc_root)
> sbi->s_proc = proc_mkdir(sb->s_id, f2fs_proc_root);
>
> @@ -1166,6 +1305,8 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
> put_stat_kobj:
> kobject_put(&sbi->s_stat_kobj);
> wait_for_completion(&sbi->s_stat_kobj_unregister);
> + kobject_put(&sbi->s_disk_feat_kobj);
> + wait_for_completion(&sbi->s_disk_feat_kobj_unregister);
It seems this should go to its own label.
Also, please note that it's very easy to get confused between
/sys/fs/f2fs/features/, /sys/fs/f2fs/$s_id/features, and
/sys/fs/f2fs/$s_id/feature_list/. Adding some comments could clarify things a
lot.
- Eric
On 06/03, Eric Biggers wrote:
> On Thu, Jun 03, 2021 at 03:08:34PM -0700, Jaegeuk Kim wrote:
> > Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > Documentation/ABI/testing/sysfs-fs-f2fs | 18 ++-
> > fs/f2fs/f2fs.h | 3 +
> > fs/f2fs/sysfs.c | 152 +++++++++++++++++++++++-
> > 3 files changed, 168 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> > index 5088281e312e..43b2cde80b70 100644
> > --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> > @@ -203,7 +203,23 @@ Description: Shows total written kbytes issued to disk.
> > What: /sys/fs/f2fs/<disk>/features
> > Date: July 2017
> > Contact: "Jaegeuk Kim" <[email protected]>
> > -Description: Shows all enabled features in current device.
> > +Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
> > + Shows all enabled features in current device.
> > + Supported features:
> > + encryption, blkzoned, extra_attr, projquota, inode_checksum,
> > + flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
> > + verity, sb_checksum, casefold, readonly, compression,
> > + encrypted_casefold, pin_file.
>
> Isn't pin_file a feature of the implementation, not of a particular filesystem
> instance? I.e. something that should go in /sys/fs/f2fs/features/, not here.
Done.
>
> Likewise for encrypted_casefold, as it is implied by encryption && casefold.
I think encrypted_casefold needs to be in both places:
1) kernel support by kconfig, 2) on-disk support by feature support.
>
> > +
> > +What: /sys/fs/f2fs/<disk>/feature_list/
> > +Date: June 2021
> > +Contact: "Jaegeuk Kim" <[email protected]>
> > +Description: Expand /sys/fs/f2fs/<disk>/features to meet sysfs rule.
> > + Supported features:
> > + encryption, block_zoned, extra_attr, projquota, inode_checksum,
> > + flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
> > + verity, sb_checksum, casefold, readonly, compression,
> > + encrypted_casefold, pin_file.
>
> Is it intentional that the file has "blkzoned" but the directory has
> "blk_zoned"?
I tried to sync this with /sys/fs/f2fs/features/block_zoned. Let me add
a comment in the doc.
>
> Also, your code has another difference -- "project_quota" is used instead of
> "projquota". But that's not mentioned above.
Ditto.
>
> And encrypted_casefold and pin_file don't seem appropriate to include here, as
> mentioned above.
Done.
>
> > @@ -1665,6 +1665,9 @@ struct f2fs_sb_info {
> > struct kobject s_stat_kobj; /* /sys/fs/f2fs/<devname>/stat */
> > struct completion s_stat_kobj_unregister;
> >
> > + struct kobject s_disk_feat_kobj; /* /sys/fs/f2fs/<devname>/feature_list */
> > + struct completion s_disk_feat_kobj_unregister;
>
> This is for a particular filesystem instance, not a disk per se. (A f2fs
> filesystem can use multiple disks.) So having "disk" in the name doesn't make
> sense.
>
> Please use more logical names like s_feature_list_kobj,
> f2fs_sb_feature_list_ktype, f2fs_sb_feat_attrs, etc.
Done.
>
> > static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> > struct f2fs_sb_info *sbi, char *buf)
> > {
> > + unsigned long feat_supp = 0;
> > +
> > switch (a->id) {
> > case FEAT_CRYPTO:
> > + feat_supp |= f2fs_sb_has_encrypt(sbi) ?
> > + (1 << FEAT_CRYPTO) : 0;
> > + fallthrough;
> > case FEAT_BLKZONED:
> > - case FEAT_ATOMIC_WRITE:
> > + feat_supp |= f2fs_sb_has_blkzoned(sbi) ?
> > + (1 << FEAT_BLKZONED) : 0;
> > + fallthrough;
> > case FEAT_EXTRA_ATTR:
> > + feat_supp |= f2fs_sb_has_extra_attr(sbi) ?
> > + (1 << FEAT_EXTRA_ATTR) : 0;
> > + fallthrough;
> > case FEAT_PROJECT_QUOTA:
> > + feat_supp |= f2fs_sb_has_project_quota(sbi) ?
> > + (1 << FEAT_PROJECT_QUOTA) : 0;
> > + fallthrough;
> > case FEAT_INODE_CHECKSUM:
> > + feat_supp |= f2fs_sb_has_inode_chksum(sbi) ?
> > + (1 << FEAT_INODE_CHECKSUM) : 0;
> > + fallthrough;
> > case FEAT_FLEXIBLE_INLINE_XATTR:
> > + feat_supp |= f2fs_sb_has_flexible_inline_xattr(sbi) ?
> > + (1 << FEAT_FLEXIBLE_INLINE_XATTR) : 0;
> > + fallthrough;
> > case FEAT_QUOTA_INO:
> > + feat_supp |= f2fs_sb_has_quota_ino(sbi) ?
> > + (1 << FEAT_QUOTA_INO) : 0;
> > + fallthrough;
> > case FEAT_INODE_CRTIME:
> > + feat_supp |= f2fs_sb_has_inode_crtime(sbi) ?
> > + (1 << FEAT_INODE_CRTIME) : 0;
> > + fallthrough;
> > case FEAT_LOST_FOUND:
> > + feat_supp |= f2fs_sb_has_lost_found(sbi) ?
> > + (1 << FEAT_LOST_FOUND) : 0;
> > + fallthrough;
> > case FEAT_VERITY:
> > + feat_supp |= f2fs_sb_has_verity(sbi) ?
> > + (1 << FEAT_VERITY) : 0;
> > + fallthrough;
> > case FEAT_SB_CHECKSUM:
> > + feat_supp |= f2fs_sb_has_sb_chksum(sbi) ?
> > + (1 << FEAT_SB_CHECKSUM) : 0;
> > + fallthrough;
> > case FEAT_CASEFOLD:
> > + feat_supp |= f2fs_sb_has_casefold(sbi) ?
> > + (1 << FEAT_CASEFOLD) : 0;
> > + fallthrough;
> > case FEAT_COMPRESSION:
> > + feat_supp |= f2fs_sb_has_compression(sbi) ?
> > + (1 << FEAT_COMPRESSION) : 0;
> > + fallthrough;
> > case FEAT_RO:
> > - case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> > + feat_supp |= f2fs_sb_has_readonly(sbi) ?
> > + (1 << FEAT_RO) : 0;
> > + fallthrough;
> > case FEAT_ENCRYPTED_CASEFOLD:
> > - return sprintf(buf, "supported\n");
> > + feat_supp |= (f2fs_sb_has_casefold(sbi) &&
> > + f2fs_sb_has_encrypt(sbi)) ?
> > + (1 << FEAT_ENCRYPTED_CASEFOLD) : 0;
> > + fallthrough;
> > + case FEAT_PIN_FILE:
> > + feat_supp |= (1 << FEAT_PIN_FILE);
> > + fallthrough;
> > + case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> > + case FEAT_ATOMIC_WRITE:
> > + if (!a->offset || feat_supp & (1 << a->id))
> > + return sprintf(buf, "supported\n");
> > }
> > - return 0;
> > + return sprintf(buf, "not supported\n");
> > }
>
> This function doesn't make much sense.
>
> Part of the problem is that the same function is handling both
> /sys/fs/f2fs/features/ and /sys/fs/f2fs/$s_id/feature_list/.
>
> All the former needs is to print "supported", since unsupported is indicated by
> the file not being there at all. So it should simply have its own ->show
> function separate from the one for feature_list/.
>
> And the feature_list/ ones could just store the F2FS_FEATURE_* bit in
> f2fs_attr::id and check for it using F2FS_HAS_FEATURE(). That would be much
> simpler -- no need for the feat_id enum or the long switch statement.
>
> Also for feature_list/ it might be better to use "unsupported" than
> "not supported", so that \<supported\> doesn't match...
Done.
>
> > static struct f2fs_attr f2fs_attr_##_name = { \
> > .attr = {.name = __stringify(_name), .mode = 0444 }, \
> > .show = f2fs_feature_show, \
> > + .offset = 0, \
> > .id = _id, \
> > }
>
> There's no need to use the .offset argument if features/ and $s_id/feature_list/
> just used different ->show functions.
>
> > +#define F2FS_DISK_FEATURE_RO_ATTR(_name, _id) \
>
> F2FS_SB_FEATURE_ATTR would be a much better name, since these pertain to a
> filesystem instance (not necessarily a disk), and all the feature attributes are
> read-only.
Used F2FS_SB_FEATURE_RO_ATTR.
>
> > static int __maybe_unused segment_info_seq_show(struct seq_file *seq,
> > void *offset)
> > {
> > @@ -1149,6 +1279,15 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
> > if (err)
> > goto put_stat_kobj;
> >
> > + sbi->s_disk_feat_kobj.kset = &f2fs_kset;
> > + init_completion(&sbi->s_disk_feat_kobj_unregister);
> > + err = kobject_init_and_add(&sbi->s_disk_feat_kobj,
> > + &f2fs_disk_feat_ktype,
> > + &sbi->s_kobj, "feature_list");
> > + if (err)
> > + goto put_stat_kobj;
> > +
> > +
> > if (f2fs_proc_root)
> > sbi->s_proc = proc_mkdir(sb->s_id, f2fs_proc_root);
> >
> > @@ -1166,6 +1305,8 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
> > put_stat_kobj:
> > kobject_put(&sbi->s_stat_kobj);
> > wait_for_completion(&sbi->s_stat_kobj_unregister);
> > + kobject_put(&sbi->s_disk_feat_kobj);
> > + wait_for_completion(&sbi->s_disk_feat_kobj_unregister);
>
> It seems this should go to its own label.
>
> Also, please note that it's very easy to get confused between
> /sys/fs/f2fs/features/, /sys/fs/f2fs/$s_id/features, and
> /sys/fs/f2fs/$s_id/feature_list/. Adding some comments could clarify things a
> lot.
Done.
>
> - Eric
Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.
Note that there are three feature list entries:
1) /sys/fs/f2fs/features
: shows runtime features supported by in-kernel f2fs along with Kconfig
- ref. F2FS_FEATURE_RO_ATTR()
2) /sys/fs/f2fs/$s_id/features <deprecated>
: shows on-disk features enabled by mkfs.f2fs, used for old kernels
3) /sys/fs/f2fs/$s_id/feature_list
: shows on-disk features enabled by mkfs.f2fs per instance.
this list covers old feature list provided by 2) and beyond
- ref. F2FS_SB_FEATURE_RO_ATTR()
Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 29 +++++-
fs/f2fs/f2fs.h | 5 +
fs/f2fs/sysfs.c | 118 ++++++++++++++++++++++++
3 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 5088281e312e..86d5e0e12a07 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -203,7 +203,34 @@ Description: Shows total written kbytes issued to disk.
What: /sys/fs/f2fs/<disk>/features
Date: July 2017
Contact: "Jaegeuk Kim" <[email protected]>
-Description: Shows all enabled features in current device.
+Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
+ Shows all enabled features in current device.
+ Supported features:
+ encryption, blkzoned, extra_attr, projquota, inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression,
+ encrypted_casefold, pin_file.
+
+What: /sys/fs/f2fs/<disk>/feature_list/
+Date: June 2021
+Contact: "Jaegeuk Kim" <[email protected]>
+Description: Expand /sys/fs/f2fs/<disk>/features to meet sysfs rule.
+ Supported on-disk features:
+ encryption, block_zoned (aka blkzoned), extra_attr,
+ project_quota (aka projquota), inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression.
+ Note that, encrypted_casefold and pin_file were moved into
+ /sys/fs/f2fs/features/.
+
+What: /sys/fs/f2fs/feature/
+Date: July 2017
+Contact: "Jaegeuk Kim" <[email protected]>
+Description: Shows all enabled kernel features.
+ Supported features:
+ in addition to all in /sys/fs/f2fs/<disk>/feature_list/,
+ test_dummy_encryption_v2, encrypted_casefold, pin_file,
+ atomic_write
What: /sys/fs/f2fs/<disk>/inject_rate
Date: May 2016
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8903c43091f8..c8c29effd844 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -172,6 +172,8 @@ struct f2fs_mount_info {
#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
+#define F2FS_MATCH_FEATURE(sbi, mask) \
+ ((sbi->raw_super->feature & cpu_to_le32(mask)) == cpu_to_le32(mask))
#define F2FS_HAS_FEATURE(sbi, mask) __F2FS_HAS_FEATURE(sbi->raw_super, mask)
#define F2FS_SET_FEATURE(sbi, mask) \
(sbi->raw_super->feature |= cpu_to_le32(mask))
@@ -1665,6 +1667,9 @@ struct f2fs_sb_info {
struct kobject s_stat_kobj; /* /sys/fs/f2fs/<devname>/stat */
struct completion s_stat_kobj_unregister;
+ struct kobject s_feature_list_kobj; /* /sys/fs/f2fs/<devname>/feature_list */
+ struct completion s_feature_list_kobj_unregister;
+
/* For shrinker support */
struct list_head s_list;
int s_ndevs; /* number of devices */
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 2a76c959a7b4..f1d8b151cc36 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -569,6 +569,20 @@ static void f2fs_sb_release(struct kobject *kobj)
complete(&sbi->s_kobj_unregister);
}
+/*
+ * Note that there are three feature list entries:
+ * 1) /sys/fs/f2fs/features
+ * : shows runtime features supported by in-kernel f2fs along with Kconfig
+ * - ref. F2FS_FEATURE_RO_ATTR()
+ *
+ * 2) /sys/fs/f2fs/$s_id/features <deprecated>
+ * : shows on-disk features enabled by mkfs.f2fs, used for old kernels
+ *
+ * 3) /sys/fs/f2fs/$s_id/feature_list
+ * : shows on-disk features enabled by mkfs.f2fs per instance.
+ * this list covers old feature list provided by 2) and beyond
+ * - ref. F2FS_SB_FEATURE_RO_ATTR()
+ */
enum feat_id {
FEAT_CRYPTO = 0,
FEAT_BLKZONED,
@@ -587,6 +601,7 @@ enum feat_id {
FEAT_RO,
FEAT_TEST_DUMMY_ENCRYPTION_V2,
FEAT_ENCRYPTED_CASEFOLD,
+ FEAT_PIN_FILE,
};
static ssize_t f2fs_feature_show(struct f2fs_attr *a,
@@ -610,6 +625,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_RO:
case FEAT_TEST_DUMMY_ENCRYPTION_V2:
case FEAT_ENCRYPTED_CASEFOLD:
+ case FEAT_PIN_FILE:
return sprintf(buf, "supported\n");
}
return 0;
@@ -743,6 +759,7 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
#endif
+F2FS_FEATURE_RO_ATTR(pin_file, FEAT_PIN_FILE);
/* For ATGC */
F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_ratio, candidate_ratio);
@@ -856,6 +873,7 @@ static struct attribute *f2fs_feat_attrs[] = {
#ifdef CONFIG_F2FS_FS_COMPRESSION
ATTR_LIST(compression),
#endif
+ ATTR_LIST(pin_file),
NULL,
};
ATTRIBUTE_GROUPS(f2fs_feat);
@@ -867,6 +885,64 @@ static struct attribute *f2fs_stat_attrs[] = {
};
ATTRIBUTE_GROUPS(f2fs_stat);
+static ssize_t f2fs_sb_feature_show(struct f2fs_attr *a,
+ struct f2fs_sb_info *sbi, char *buf)
+{
+ if (F2FS_MATCH_FEATURE(sbi, a->id))
+ return sprintf(buf, "supported\n");
+ return sprintf(buf, "unsupported\n");
+}
+
+#define F2FS_SB_FEATURE_RO_ATTR(_name, _feat) \
+static struct f2fs_attr f2fs_attr_sb_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_sb_feature_show, \
+ .id = F2FS_FEATURE_##_feat, \
+}
+
+#define F2FS_SB_FEATURE_RO_ATTR2(_name, _feat1, _feat2) \
+static struct f2fs_attr f2fs_attr_sb_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_sb_feature_show, \
+ .id = F2FS_FEATURE_##_feat1 | F2FS_FEATURE_##_feat2,\
+}
+
+F2FS_SB_FEATURE_RO_ATTR(encryption, ENCRYPT);
+F2FS_SB_FEATURE_RO_ATTR(block_zoned, BLKZONED);
+F2FS_SB_FEATURE_RO_ATTR(extra_attr, EXTRA_ATTR);
+F2FS_SB_FEATURE_RO_ATTR(project_quota, PRJQUOTA);
+F2FS_SB_FEATURE_RO_ATTR(inode_checksum, INODE_CHKSUM);
+F2FS_SB_FEATURE_RO_ATTR(flexible_inline_xattr, FLEXIBLE_INLINE_XATTR);
+F2FS_SB_FEATURE_RO_ATTR(quota_ino, QUOTA_INO);
+F2FS_SB_FEATURE_RO_ATTR(inode_crtime, INODE_CRTIME);
+F2FS_SB_FEATURE_RO_ATTR(lost_found, LOST_FOUND);
+F2FS_SB_FEATURE_RO_ATTR(verity, VERITY);
+F2FS_SB_FEATURE_RO_ATTR(sb_checksum, SB_CHKSUM);
+F2FS_SB_FEATURE_RO_ATTR(casefold, CASEFOLD);
+F2FS_SB_FEATURE_RO_ATTR(compression, COMPRESSION);
+F2FS_SB_FEATURE_RO_ATTR(readonly, RO);
+F2FS_SB_FEATURE_RO_ATTR2(encrypted_casefold, ENCRYPT, CASEFOLD);
+
+static struct attribute *f2fs_sb_feat_attrs[] = {
+ ATTR_LIST(sb_encryption),
+ ATTR_LIST(sb_block_zoned),
+ ATTR_LIST(sb_extra_attr),
+ ATTR_LIST(sb_project_quota),
+ ATTR_LIST(sb_inode_checksum),
+ ATTR_LIST(sb_flexible_inline_xattr),
+ ATTR_LIST(sb_quota_ino),
+ ATTR_LIST(sb_inode_crtime),
+ ATTR_LIST(sb_lost_found),
+ ATTR_LIST(sb_verity),
+ ATTR_LIST(sb_sb_checksum),
+ ATTR_LIST(sb_casefold),
+ ATTR_LIST(sb_compression),
+ ATTR_LIST(sb_readonly),
+ ATTR_LIST(sb_encrypted_casefold),
+ NULL,
+};
+ATTRIBUTE_GROUPS(f2fs_sb_feat);
+
static const struct sysfs_ops f2fs_attr_ops = {
.show = f2fs_attr_show,
.store = f2fs_attr_store,
@@ -933,6 +1009,34 @@ static struct kobj_type f2fs_stat_ktype = {
.release = f2fs_stat_kobj_release,
};
+static ssize_t f2fs_sb_feat_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_feature_list_kobj);
+ struct f2fs_attr *a = container_of(attr, struct f2fs_attr, attr);
+
+ return a->show ? a->show(a, sbi, buf) : 0;
+}
+
+static void f2fs_feature_list_kobj_release(struct kobject *kobj)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_feature_list_kobj);
+ complete(&sbi->s_feature_list_kobj_unregister);
+}
+
+static const struct sysfs_ops f2fs_feature_list_attr_ops = {
+ .show = f2fs_sb_feat_attr_show,
+};
+
+static struct kobj_type f2fs_feature_list_ktype = {
+ .default_groups = f2fs_sb_feat_groups,
+ .sysfs_ops = &f2fs_feature_list_attr_ops,
+ .release = f2fs_feature_list_kobj_release,
+};
+
+
static int __maybe_unused segment_info_seq_show(struct seq_file *seq,
void *offset)
{
@@ -1149,6 +1253,14 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
if (err)
goto put_stat_kobj;
+ sbi->s_feature_list_kobj.kset = &f2fs_kset;
+ init_completion(&sbi->s_feature_list_kobj_unregister);
+ err = kobject_init_and_add(&sbi->s_feature_list_kobj,
+ &f2fs_feature_list_ktype,
+ &sbi->s_kobj, "feature_list");
+ if (err)
+ goto put_feature_list_kobj;
+
if (f2fs_proc_root)
sbi->s_proc = proc_mkdir(sb->s_id, f2fs_proc_root);
@@ -1163,6 +1275,9 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
victim_bits_seq_show, sb);
}
return 0;
+put_feature_list_kobj:
+ kobject_put(&sbi->s_feature_list_kobj);
+ wait_for_completion(&sbi->s_feature_list_kobj_unregister);
put_stat_kobj:
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
@@ -1185,6 +1300,9 @@ void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
kobject_del(&sbi->s_stat_kobj);
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
+ kobject_del(&sbi->s_feature_list_kobj);
+ kobject_put(&sbi->s_feature_list_kobj);
+ wait_for_completion(&sbi->s_feature_list_kobj_unregister);
kobject_del(&sbi->s_kobj);
kobject_put(&sbi->s_kobj);
--
2.32.0.rc1.229.g3e70b5a671-goog
On Thu, Jun 03, 2021 at 09:42:08PM -0700, Jaegeuk Kim wrote:
> enum feat_id {
> FEAT_CRYPTO = 0,
> FEAT_BLKZONED,
> @@ -587,6 +601,7 @@ enum feat_id {
> FEAT_RO,
> FEAT_TEST_DUMMY_ENCRYPTION_V2,
> FEAT_ENCRYPTED_CASEFOLD,
> + FEAT_PIN_FILE,
> };
>
> static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> @@ -610,6 +625,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> case FEAT_RO:
> case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> case FEAT_ENCRYPTED_CASEFOLD:
> + case FEAT_PIN_FILE:
> return sprintf(buf, "supported\n");
> }
> return 0;
There's no need for the feat_id enum to exist. If f2fs_feature_show() just
always printed "supported\n", it will do the right thing.
Also, adding pin_file probably should be a separate patch. That seems to be a
bug fix, as pin_file was mistakenly added to the per-sb feature list instead of
to the kernel feature list?
> +static ssize_t f2fs_sb_feature_show(struct f2fs_attr *a,
> + struct f2fs_sb_info *sbi, char *buf)
> +{
> + if (F2FS_MATCH_FEATURE(sbi, a->id))
> + return sprintf(buf, "supported\n");
> + return sprintf(buf, "unsupported\n");
> +}
This can just use F2FS_HAS_FEATURE(), provided that encrypted_casefold isn't
included here, which it shouldn't be (as discussed elsewhere).
- Eric
On 06/03, Eric Biggers wrote:
> On Thu, Jun 03, 2021 at 09:42:08PM -0700, Jaegeuk Kim wrote:
> > enum feat_id {
> > FEAT_CRYPTO = 0,
> > FEAT_BLKZONED,
> > @@ -587,6 +601,7 @@ enum feat_id {
> > FEAT_RO,
> > FEAT_TEST_DUMMY_ENCRYPTION_V2,
> > FEAT_ENCRYPTED_CASEFOLD,
> > + FEAT_PIN_FILE,
> > };
> >
> > static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> > @@ -610,6 +625,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> > case FEAT_RO:
> > case FEAT_TEST_DUMMY_ENCRYPTION_V2:
> > case FEAT_ENCRYPTED_CASEFOLD:
> > + case FEAT_PIN_FILE:
> > return sprintf(buf, "supported\n");
> > }
> > return 0;
>
> There's no need for the feat_id enum to exist. If f2fs_feature_show() just
> always printed "supported\n", it will do the right thing.
Done.
>
> Also, adding pin_file probably should be a separate patch. That seems to be a
> bug fix, as pin_file was mistakenly added to the per-sb feature list instead of
> to the kernel feature list?
Done.
>
> > +static ssize_t f2fs_sb_feature_show(struct f2fs_attr *a,
> > + struct f2fs_sb_info *sbi, char *buf)
> > +{
> > + if (F2FS_MATCH_FEATURE(sbi, a->id))
> > + return sprintf(buf, "supported\n");
> > + return sprintf(buf, "unsupported\n");
> > +}
>
> This can just use F2FS_HAS_FEATURE(), provided that encrypted_casefold isn't
> included here, which it shouldn't be (as discussed elsewhere).
I know, but I think it'd be good to sync with kernel feature.
>
> - Eric
Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.
Note that there are three feature list entries:
1) /sys/fs/f2fs/features
: shows runtime features supported by in-kernel f2fs along with Kconfig
- ref. F2FS_FEATURE_RO_ATTR()
2) /sys/fs/f2fs/$s_id/features <deprecated>
: shows on-disk features enabled by mkfs.f2fs, used for old kernels
3) /sys/fs/f2fs/$s_id/feature_list
: shows on-disk features enabled by mkfs.f2fs per instance.
this list covers old feature list provided by 2) and beyond
- ref. F2FS_SB_FEATURE_RO_ATTR()
Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 28 +++-
fs/f2fs/f2fs.h | 5 +
fs/f2fs/sysfs.c | 200 ++++++++++++++++--------
3 files changed, 169 insertions(+), 64 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 5088281e312e..ca697cc1e765 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -203,7 +203,33 @@ Description: Shows total written kbytes issued to disk.
What: /sys/fs/f2fs/<disk>/features
Date: July 2017
Contact: "Jaegeuk Kim" <[email protected]>
-Description: Shows all enabled features in current device.
+Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
+ Shows all enabled features in current device.
+ Supported features:
+ encryption, blkzoned, extra_attr, projquota, inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression,
+ encrypted_casefold, pin_file.
+
+What: /sys/fs/f2fs/<disk>/feature_list/
+Date: June 2021
+Contact: "Jaegeuk Kim" <[email protected]>
+Description: Expand /sys/fs/f2fs/<disk>/features to meet sysfs rule.
+ Supported on-disk features:
+ encryption, block_zoned (aka blkzoned), extra_attr,
+ project_quota (aka projquota), inode_checksum,
+ flexible_inline_xattr, quota_ino, inode_crtime, lost_found,
+ verity, sb_checksum, casefold, readonly, compression,
+ encrypted_casefold.
+ Note that, pin_file is moved into /sys/fs/f2fs/features/.
+
+What: /sys/fs/f2fs/feature/
+Date: July 2017
+Contact: "Jaegeuk Kim" <[email protected]>
+Description: Shows all enabled kernel features.
+ Supported features:
+ in addition to all in /sys/fs/f2fs/<disk>/feature_list/,
+ test_dummy_encryption_v2, atomic_write, pin_file
What: /sys/fs/f2fs/<disk>/inject_rate
Date: May 2016
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8903c43091f8..c8c29effd844 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -172,6 +172,8 @@ struct f2fs_mount_info {
#define __F2FS_HAS_FEATURE(raw_super, mask) \
((raw_super->feature & cpu_to_le32(mask)) != 0)
+#define F2FS_MATCH_FEATURE(sbi, mask) \
+ ((sbi->raw_super->feature & cpu_to_le32(mask)) == cpu_to_le32(mask))
#define F2FS_HAS_FEATURE(sbi, mask) __F2FS_HAS_FEATURE(sbi->raw_super, mask)
#define F2FS_SET_FEATURE(sbi, mask) \
(sbi->raw_super->feature |= cpu_to_le32(mask))
@@ -1665,6 +1667,9 @@ struct f2fs_sb_info {
struct kobject s_stat_kobj; /* /sys/fs/f2fs/<devname>/stat */
struct completion s_stat_kobj_unregister;
+ struct kobject s_feature_list_kobj; /* /sys/fs/f2fs/<devname>/feature_list */
+ struct completion s_feature_list_kobj_unregister;
+
/* For shrinker support */
struct list_head s_list;
int s_ndevs; /* number of devices */
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index a1d3ba75c753..1e9e1ea8d4f3 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -569,50 +569,53 @@ static void f2fs_sb_release(struct kobject *kobj)
complete(&sbi->s_kobj_unregister);
}
-enum feat_id {
- FEAT_CRYPTO = 0,
- FEAT_BLKZONED,
- FEAT_ATOMIC_WRITE,
- FEAT_EXTRA_ATTR,
- FEAT_PROJECT_QUOTA,
- FEAT_INODE_CHECKSUM,
- FEAT_FLEXIBLE_INLINE_XATTR,
- FEAT_QUOTA_INO,
- FEAT_INODE_CRTIME,
- FEAT_LOST_FOUND,
- FEAT_VERITY,
- FEAT_SB_CHECKSUM,
- FEAT_CASEFOLD,
- FEAT_COMPRESSION,
- FEAT_RO,
- FEAT_TEST_DUMMY_ENCRYPTION_V2,
- FEAT_ENCRYPTED_CASEFOLD,
-};
+/*
+ * Note that there are three feature list entries:
+ * 1) /sys/fs/f2fs/features
+ * : shows runtime features supported by in-kernel f2fs along with Kconfig
+ * - ref. F2FS_FEATURE_RO_ATTR()
+ *
+ * 2) /sys/fs/f2fs/$s_id/features <deprecated>
+ * : shows on-disk features enabled by mkfs.f2fs, used for old kernels
+ *
+ * 3) /sys/fs/f2fs/$s_id/feature_list
+ * : shows on-disk features enabled by mkfs.f2fs per instance.
+ * this list covers old feature list provided by 2) and beyond
+ * - ref. F2FS_SB_FEATURE_RO_ATTR()
+ */
static ssize_t f2fs_feature_show(struct f2fs_attr *a,
struct f2fs_sb_info *sbi, char *buf)
{
- switch (a->id) {
- case FEAT_CRYPTO:
- case FEAT_BLKZONED:
- case FEAT_ATOMIC_WRITE:
- case FEAT_EXTRA_ATTR:
- case FEAT_PROJECT_QUOTA:
- case FEAT_INODE_CHECKSUM:
- case FEAT_FLEXIBLE_INLINE_XATTR:
- case FEAT_QUOTA_INO:
- case FEAT_INODE_CRTIME:
- case FEAT_LOST_FOUND:
- case FEAT_VERITY:
- case FEAT_SB_CHECKSUM:
- case FEAT_CASEFOLD:
- case FEAT_COMPRESSION:
- case FEAT_RO:
- case FEAT_TEST_DUMMY_ENCRYPTION_V2:
- case FEAT_ENCRYPTED_CASEFOLD:
+ return sprintf(buf, "supported\n");
+}
+
+#define F2FS_FEATURE_RO_ATTR(_name) \
+static struct f2fs_attr f2fs_attr_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_feature_show, \
+}
+
+static ssize_t f2fs_sb_feature_show(struct f2fs_attr *a,
+ struct f2fs_sb_info *sbi, char *buf)
+{
+ if (F2FS_MATCH_FEATURE(sbi, a->id))
return sprintf(buf, "supported\n");
- }
- return 0;
+ return sprintf(buf, "unsupported\n");
+}
+
+#define F2FS_SB_FEATURE_RO_ATTR(_name, _feat) \
+static struct f2fs_attr f2fs_attr_sb_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_sb_feature_show, \
+ .id = F2FS_FEATURE_##_feat, \
+}
+
+#define F2FS_SB_FEATURE_RO_ATTR2(_name, _feat1, _feat2) \
+static struct f2fs_attr f2fs_attr_sb_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = 0444 }, \
+ .show = f2fs_sb_feature_show, \
+ .id = F2FS_FEATURE_##_feat1 | F2FS_FEATURE_##_feat2,\
}
#define F2FS_ATTR_OFFSET(_struct_type, _name, _mode, _show, _store, _offset) \
@@ -632,13 +635,6 @@ static struct f2fs_attr f2fs_attr_##_name = { \
#define F2FS_GENERAL_RO_ATTR(name) \
static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
-#define F2FS_FEATURE_RO_ATTR(_name, _id) \
-static struct f2fs_attr f2fs_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = 0444 }, \
- .show = f2fs_feature_show, \
- .id = _id, \
-}
-
#define F2FS_STAT_ATTR(_struct_type, _struct_name, _name, _elname) \
static struct f2fs_attr f2fs_attr_##_name = { \
.attr = {.name = __stringify(_name), .mode = 0444 }, \
@@ -712,33 +708,33 @@ F2FS_GENERAL_RO_ATTR(avg_vblocks);
#endif
#ifdef CONFIG_FS_ENCRYPTION
-F2FS_FEATURE_RO_ATTR(encryption, FEAT_CRYPTO);
-F2FS_FEATURE_RO_ATTR(test_dummy_encryption_v2, FEAT_TEST_DUMMY_ENCRYPTION_V2);
+F2FS_FEATURE_RO_ATTR(encryption);
+F2FS_FEATURE_RO_ATTR(test_dummy_encryption_v2);
#ifdef CONFIG_UNICODE
-F2FS_FEATURE_RO_ATTR(encrypted_casefold, FEAT_ENCRYPTED_CASEFOLD);
+F2FS_FEATURE_RO_ATTR(encrypted_casefold);
#endif /* CONFIG_UNICODE */
#endif /* CONFIG_FS_ENCRYPTION */
#ifdef CONFIG_BLK_DEV_ZONED
-F2FS_FEATURE_RO_ATTR(block_zoned, FEAT_BLKZONED);
+F2FS_FEATURE_RO_ATTR(block_zoned);
#endif
-F2FS_FEATURE_RO_ATTR(atomic_write, FEAT_ATOMIC_WRITE);
-F2FS_FEATURE_RO_ATTR(extra_attr, FEAT_EXTRA_ATTR);
-F2FS_FEATURE_RO_ATTR(project_quota, FEAT_PROJECT_QUOTA);
-F2FS_FEATURE_RO_ATTR(inode_checksum, FEAT_INODE_CHECKSUM);
-F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
-F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
-F2FS_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
-F2FS_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
+F2FS_FEATURE_RO_ATTR(atomic_write);
+F2FS_FEATURE_RO_ATTR(extra_attr);
+F2FS_FEATURE_RO_ATTR(project_quota);
+F2FS_FEATURE_RO_ATTR(inode_checksum);
+F2FS_FEATURE_RO_ATTR(flexible_inline_xattr);
+F2FS_FEATURE_RO_ATTR(quota_ino);
+F2FS_FEATURE_RO_ATTR(inode_crtime);
+F2FS_FEATURE_RO_ATTR(lost_found);
#ifdef CONFIG_FS_VERITY
-F2FS_FEATURE_RO_ATTR(verity, FEAT_VERITY);
+F2FS_FEATURE_RO_ATTR(verity);
#endif
-F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
+F2FS_FEATURE_RO_ATTR(sb_checksum);
#ifdef CONFIG_UNICODE
-F2FS_FEATURE_RO_ATTR(casefold, FEAT_CASEFOLD);
+F2FS_FEATURE_RO_ATTR(casefold);
#endif
-F2FS_FEATURE_RO_ATTR(readonly, FEAT_RO);
+F2FS_FEATURE_RO_ATTR(readonly);
#ifdef CONFIG_F2FS_FS_COMPRESSION
-F2FS_FEATURE_RO_ATTR(compression, FEAT_COMPRESSION);
+F2FS_FEATURE_RO_ATTR(compression);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_written_block, compr_written_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_saved_block, compr_saved_block);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, compr_new_inode, compr_new_inode);
@@ -869,6 +865,42 @@ static struct attribute *f2fs_stat_attrs[] = {
};
ATTRIBUTE_GROUPS(f2fs_stat);
+F2FS_SB_FEATURE_RO_ATTR(encryption, ENCRYPT);
+F2FS_SB_FEATURE_RO_ATTR(block_zoned, BLKZONED);
+F2FS_SB_FEATURE_RO_ATTR(extra_attr, EXTRA_ATTR);
+F2FS_SB_FEATURE_RO_ATTR(project_quota, PRJQUOTA);
+F2FS_SB_FEATURE_RO_ATTR(inode_checksum, INODE_CHKSUM);
+F2FS_SB_FEATURE_RO_ATTR(flexible_inline_xattr, FLEXIBLE_INLINE_XATTR);
+F2FS_SB_FEATURE_RO_ATTR(quota_ino, QUOTA_INO);
+F2FS_SB_FEATURE_RO_ATTR(inode_crtime, INODE_CRTIME);
+F2FS_SB_FEATURE_RO_ATTR(lost_found, LOST_FOUND);
+F2FS_SB_FEATURE_RO_ATTR(verity, VERITY);
+F2FS_SB_FEATURE_RO_ATTR(sb_checksum, SB_CHKSUM);
+F2FS_SB_FEATURE_RO_ATTR(casefold, CASEFOLD);
+F2FS_SB_FEATURE_RO_ATTR(compression, COMPRESSION);
+F2FS_SB_FEATURE_RO_ATTR(readonly, RO);
+F2FS_SB_FEATURE_RO_ATTR2(encrypted_casefold, ENCRYPT, CASEFOLD);
+
+static struct attribute *f2fs_sb_feat_attrs[] = {
+ ATTR_LIST(sb_encryption),
+ ATTR_LIST(sb_block_zoned),
+ ATTR_LIST(sb_extra_attr),
+ ATTR_LIST(sb_project_quota),
+ ATTR_LIST(sb_inode_checksum),
+ ATTR_LIST(sb_flexible_inline_xattr),
+ ATTR_LIST(sb_quota_ino),
+ ATTR_LIST(sb_inode_crtime),
+ ATTR_LIST(sb_lost_found),
+ ATTR_LIST(sb_verity),
+ ATTR_LIST(sb_sb_checksum),
+ ATTR_LIST(sb_casefold),
+ ATTR_LIST(sb_compression),
+ ATTR_LIST(sb_readonly),
+ ATTR_LIST(sb_encrypted_casefold),
+ NULL,
+};
+ATTRIBUTE_GROUPS(f2fs_sb_feat);
+
static const struct sysfs_ops f2fs_attr_ops = {
.show = f2fs_attr_show,
.store = f2fs_attr_store,
@@ -935,6 +967,34 @@ static struct kobj_type f2fs_stat_ktype = {
.release = f2fs_stat_kobj_release,
};
+static ssize_t f2fs_sb_feat_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_feature_list_kobj);
+ struct f2fs_attr *a = container_of(attr, struct f2fs_attr, attr);
+
+ return a->show ? a->show(a, sbi, buf) : 0;
+}
+
+static void f2fs_feature_list_kobj_release(struct kobject *kobj)
+{
+ struct f2fs_sb_info *sbi = container_of(kobj, struct f2fs_sb_info,
+ s_feature_list_kobj);
+ complete(&sbi->s_feature_list_kobj_unregister);
+}
+
+static const struct sysfs_ops f2fs_feature_list_attr_ops = {
+ .show = f2fs_sb_feat_attr_show,
+};
+
+static struct kobj_type f2fs_feature_list_ktype = {
+ .default_groups = f2fs_sb_feat_groups,
+ .sysfs_ops = &f2fs_feature_list_attr_ops,
+ .release = f2fs_feature_list_kobj_release,
+};
+
+
static int __maybe_unused segment_info_seq_show(struct seq_file *seq,
void *offset)
{
@@ -1151,6 +1211,14 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
if (err)
goto put_stat_kobj;
+ sbi->s_feature_list_kobj.kset = &f2fs_kset;
+ init_completion(&sbi->s_feature_list_kobj_unregister);
+ err = kobject_init_and_add(&sbi->s_feature_list_kobj,
+ &f2fs_feature_list_ktype,
+ &sbi->s_kobj, "feature_list");
+ if (err)
+ goto put_feature_list_kobj;
+
if (f2fs_proc_root)
sbi->s_proc = proc_mkdir(sb->s_id, f2fs_proc_root);
@@ -1165,6 +1233,9 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
victim_bits_seq_show, sb);
}
return 0;
+put_feature_list_kobj:
+ kobject_put(&sbi->s_feature_list_kobj);
+ wait_for_completion(&sbi->s_feature_list_kobj_unregister);
put_stat_kobj:
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
@@ -1187,6 +1258,9 @@ void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
kobject_del(&sbi->s_stat_kobj);
kobject_put(&sbi->s_stat_kobj);
wait_for_completion(&sbi->s_stat_kobj_unregister);
+ kobject_del(&sbi->s_feature_list_kobj);
+ kobject_put(&sbi->s_feature_list_kobj);
+ wait_for_completion(&sbi->s_feature_list_kobj_unregister);
kobject_del(&sbi->s_kobj);
kobject_put(&sbi->s_kobj);
--
2.32.0.rc1.229.g3e70b5a671-goog