All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-03 22:08 ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-03 22:08 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim

Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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


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

* [f2fs-dev] [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-03 22:08 ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-03 22:08 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim

Let's create /sys/fs/f2fs/<disk>/feature_list/ to meet sysfs rule.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-03 22:08 ` [f2fs-dev] " Jaegeuk Kim
@ 2021-06-04  0:40   ` Eric Biggers
  -1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2021-06-04  0:40 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
> ---
>  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" <jaegeuk@kernel.org>
> -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" <jaegeuk@kernel.org>
> +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

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

* Re: [f2fs-dev] [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  0:40   ` Eric Biggers
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2021-06-04  0:40 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
> ---
>  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" <jaegeuk@kernel.org>
> -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" <jaegeuk@kernel.org>
> +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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-04  0:40   ` Eric Biggers
@ 2021-06-04  4:41     ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  4:41 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
> > ---
> >  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" <jaegeuk@kernel.org>
> > -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" <jaegeuk@kernel.org>
> > +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

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

* Re: [f2fs-dev] [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  4:41     ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  4:41 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
> > ---
> >  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" <jaegeuk@kernel.org>
> > -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" <jaegeuk@kernel.org>
> > +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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-03 22:08 ` [f2fs-dev] " Jaegeuk Kim
@ 2021-06-04  4:42   ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  4:42 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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" <jaegeuk@kernel.org>
+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


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

* Re: [f2fs-dev] [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  4:42   ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  4:42 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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" <jaegeuk@kernel.org>
+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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-04  4:42   ` [f2fs-dev] " Jaegeuk Kim
@ 2021-06-04  5:10     ` Eric Biggers
  -1 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2021-06-04  5:10 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

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

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

* Re: [f2fs-dev] [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  5:10     ` Eric Biggers
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2021-06-04  5:10 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-04  5:10     ` Eric Biggers
@ 2021-06-04  5:34       ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  5:34 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel

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

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

* Re: [f2fs-dev] [PATCH v2] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  5:34       ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  5:34 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel

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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v3] f2fs: clean up /sys/fs/f2fs/<disk>/features
  2021-06-04  4:42   ` [f2fs-dev] " Jaegeuk Kim
@ 2021-06-04  5:49     ` Jaegeuk Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  5:49 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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" <jaegeuk@kernel.org>
+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


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

* Re: [f2fs-dev] [PATCH v3] f2fs: clean up /sys/fs/f2fs/<disk>/features
@ 2021-06-04  5:49     ` Jaegeuk Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Jaegeuk Kim @ 2021-06-04  5:49 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel

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 <jaegeuk@kernel.org>
---
 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" <jaegeuk@kernel.org>
-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" <jaegeuk@kernel.org>
+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" <jaegeuk@kernel.org>
+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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, other threads:[~2021-06-04  5:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 22:08 [PATCH] f2fs: clean up /sys/fs/f2fs/<disk>/features Jaegeuk Kim
2021-06-03 22:08 ` [f2fs-dev] " Jaegeuk Kim
2021-06-04  0:40 ` Eric Biggers
2021-06-04  0:40   ` Eric Biggers
2021-06-04  4:41   ` Jaegeuk Kim
2021-06-04  4:41     ` Jaegeuk Kim
2021-06-04  4:42 ` [PATCH v2] " Jaegeuk Kim
2021-06-04  4:42   ` [f2fs-dev] " Jaegeuk Kim
2021-06-04  5:10   ` Eric Biggers
2021-06-04  5:10     ` Eric Biggers
2021-06-04  5:34     ` Jaegeuk Kim
2021-06-04  5:34       ` Jaegeuk Kim
2021-06-04  5:49   ` [f2fs-dev] [PATCH v3] " Jaegeuk Kim
2021-06-04  5:49     ` Jaegeuk Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.