All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-19  5:44 ` Daeho Jeong
  0 siblings, 0 replies; 16+ messages in thread
From: Daeho Jeong @ 2020-07-19  5:44 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Added a symbolic link directory pointing to its device name
directory using the volume name of the partition in sysfs.
(i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
"vol" is the volume name of the partition, "#x" is the number
representing the order of mounting with the same volume name.
We allow the volume name duplication within 100, which means the
range of #x should be 0 ~ 99. Once one mount point was umounted,
that sequential number #x in "vol_#x" could be reused by later
newly mounted point.

It will provide easy access to sysfs node using volume name,
even if not knowing the specific device node name like sda1 and
dm-3.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 Documentation/filesystems/f2fs.rst | 21 +++++++++++--
 fs/f2fs/f2fs.h                     | 14 +++++++++
 fs/f2fs/file.c                     |  2 ++
 fs/f2fs/super.c                    |  1 +
 fs/f2fs/sysfs.c                    | 50 ++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index de43239a3c31..8221f3af6042 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -278,8 +278,25 @@ Sysfs Entries
 =============
 
 Information about mounted f2fs file systems can be found in
-/sys/fs/f2fs.  Each mounted filesystem will have a directory in
-/sys/fs/f2fs based on its device name (i.e., /sys/fs/f2fs/sda).
+/sys/fs/f2fs. Each mounted filesystem will have a directory in
+/sys/fs/f2fs based on its device name (i.e., /sys/fs/f2fs/sda),
+or, if the partition has the volume name in the on-disk super
+block, the volume name with the number (i.e., /sys/fs/f2fs/vol_#x).
+"vol" is the volume name of the partition, "#x" is the number
+representing the order of mounting with the same volume name.
+We allow the volume name duplication within 100, which means the
+range of #x should be 0 ~ 99. Once one mount point was umounted,
+that sequential number #x in "vol_#x" could be reused by later
+newly mounted point.
+
+Here is an example of this.
+
+mount dev0 mount0 (vol_0 -> dev0)
+mount dev1 mount1 (vol_1 -> dev1)
+umount mount0
+mount dev2 mount2 (vol_0 -> dev2)
+* dev0, dev1 and dev2 have the same volume name "vol".
+
 The files in each per-device directory are shown in table below.
 
 Files in /sys/fs/f2fs/<devname>
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a555377cf51f..392ee7d7a37d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1382,6 +1382,10 @@ struct decompress_io_ctx {
 #define MAX_COMPRESS_LOG_SIZE		8
 #define MAX_COMPRESS_WINDOW_SIZE	((PAGE_SIZE) << MAX_COMPRESS_LOG_SIZE)
 
+#define MAX_DUP_NAME			8
+#define MAX_SYSLINK_NAME		(MAX_VOLUME_NAME + MAX_DUP_NAME)
+#define MAX_DUP_NUM			100
+
 struct f2fs_sb_info {
 	struct super_block *sb;			/* pointer to VFS super block */
 	struct proc_dir_entry *s_proc;		/* proc entry */
@@ -1586,6 +1590,10 @@ struct f2fs_sb_info {
 
 	struct kmem_cache *inline_xattr_slab;	/* inline xattr entry */
 	unsigned int inline_xattr_slab_size;	/* default inline xattr slab size */
+
+	/* For sysfs symlink per volume */
+	char syslink_name[MAX_SYSLINK_NAME];
+	struct mutex syslink_mutex;
 };
 
 struct f2fs_private_dio {
@@ -3816,6 +3824,12 @@ int __init f2fs_init_sysfs(void);
 void f2fs_exit_sysfs(void);
 int f2fs_register_sysfs(struct f2fs_sb_info *sbi);
 void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi);
+void f2fs_reload_syslink(struct f2fs_sb_info *sbi);
+
+static inline bool f2fs_has_syslink(struct f2fs_sb_info *sbi)
+{
+	return strlen(sbi->syslink_name);
+}
 
 /* verity.c */
 extern const struct fsverity_operations f2fs_verityops;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 521987cd8772..4612f645007a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3418,6 +3418,8 @@ static int f2fs_ioc_setfslabel(struct file *filp, unsigned long arg)
 
 	up_write(&sbi->sb_lock);
 
+	f2fs_reload_syslink(sbi);
+
 	mnt_drop_write_file(filp);
 out:
 	kfree(vbuf);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2686b07ae7eb..7b002785417a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3021,6 +3021,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	init_rwsem(&sbi->sb_lock);
 	init_rwsem(&sbi->pin_sem);
+	mutex_init(&sbi->syslink_mutex);
 }
 
 static int init_percpu_info(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 50524401c8e6..e9818dd338c1 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -11,6 +11,7 @@
 #include <linux/f2fs_fs.h>
 #include <linux/seq_file.h>
 #include <linux/unicode.h>
+#include <linux/nls.h>
 
 #include "f2fs.h"
 #include "segment.h"
@@ -897,6 +898,51 @@ static int __maybe_unused victim_bits_seq_show(struct seq_file *seq,
 	return 0;
 }
 
+static void f2fs_unload_syslink(struct f2fs_sb_info *sbi)
+{
+	if (!f2fs_has_syslink(sbi))
+		return;
+
+	sysfs_remove_link(&f2fs_kset.kobj, sbi->syslink_name);
+	memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
+}
+
+static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
+{
+	int idx, count, ret;
+
+	down_read(&sbi->sb_lock);
+	count = utf16s_to_utf8s(sbi->raw_super->volume_name,
+			ARRAY_SIZE(sbi->raw_super->volume_name),
+			UTF16_LITTLE_ENDIAN, sbi->syslink_name,
+			MAX_VOLUME_NAME);
+	up_read(&sbi->sb_lock);
+
+	if (!count)
+		return -ENOENT;
+
+	for (idx = 0; idx < MAX_DUP_NUM; idx++) {
+		snprintf(sbi->syslink_name + count, MAX_DUP_NAME, "_%d", idx);
+		ret = sysfs_create_link(&f2fs_kset.kobj, &sbi->s_kobj,
+				sbi->syslink_name);
+		if (ret != -EEXIST)
+			break;
+	}
+
+	if (ret)
+		memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
+
+	return ret;
+}
+
+void f2fs_reload_syslink(struct f2fs_sb_info *sbi)
+{
+	mutex_lock(&sbi->syslink_mutex);
+	f2fs_unload_syslink(sbi);
+	f2fs_load_syslink(sbi);
+	mutex_unlock(&sbi->syslink_mutex);
+}
+
 int __init f2fs_init_sysfs(void)
 {
 	int ret;
@@ -954,11 +1000,15 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
 		proc_create_single_data("victim_bits", S_IRUGO, sbi->s_proc,
 				victim_bits_seq_show, sb);
 	}
+
+	f2fs_load_syslink(sbi);
+
 	return 0;
 }
 
 void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
 {
+	f2fs_unload_syslink(sbi);
 	if (sbi->s_proc) {
 		remove_proc_entry("iostat_info", sbi->s_proc);
 		remove_proc_entry("segment_info", sbi->s_proc);
-- 
2.28.0.rc0.105.gf9edc3c819-goog


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

* [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-19  5:44 ` Daeho Jeong
  0 siblings, 0 replies; 16+ messages in thread
From: Daeho Jeong @ 2020-07-19  5:44 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Added a symbolic link directory pointing to its device name
directory using the volume name of the partition in sysfs.
(i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
"vol" is the volume name of the partition, "#x" is the number
representing the order of mounting with the same volume name.
We allow the volume name duplication within 100, which means the
range of #x should be 0 ~ 99. Once one mount point was umounted,
that sequential number #x in "vol_#x" could be reused by later
newly mounted point.

It will provide easy access to sysfs node using volume name,
even if not knowing the specific device node name like sda1 and
dm-3.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 Documentation/filesystems/f2fs.rst | 21 +++++++++++--
 fs/f2fs/f2fs.h                     | 14 +++++++++
 fs/f2fs/file.c                     |  2 ++
 fs/f2fs/super.c                    |  1 +
 fs/f2fs/sysfs.c                    | 50 ++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index de43239a3c31..8221f3af6042 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -278,8 +278,25 @@ Sysfs Entries
 =============
 
 Information about mounted f2fs file systems can be found in
-/sys/fs/f2fs.  Each mounted filesystem will have a directory in
-/sys/fs/f2fs based on its device name (i.e., /sys/fs/f2fs/sda).
+/sys/fs/f2fs. Each mounted filesystem will have a directory in
+/sys/fs/f2fs based on its device name (i.e., /sys/fs/f2fs/sda),
+or, if the partition has the volume name in the on-disk super
+block, the volume name with the number (i.e., /sys/fs/f2fs/vol_#x).
+"vol" is the volume name of the partition, "#x" is the number
+representing the order of mounting with the same volume name.
+We allow the volume name duplication within 100, which means the
+range of #x should be 0 ~ 99. Once one mount point was umounted,
+that sequential number #x in "vol_#x" could be reused by later
+newly mounted point.
+
+Here is an example of this.
+
+mount dev0 mount0 (vol_0 -> dev0)
+mount dev1 mount1 (vol_1 -> dev1)
+umount mount0
+mount dev2 mount2 (vol_0 -> dev2)
+* dev0, dev1 and dev2 have the same volume name "vol".
+
 The files in each per-device directory are shown in table below.
 
 Files in /sys/fs/f2fs/<devname>
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a555377cf51f..392ee7d7a37d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1382,6 +1382,10 @@ struct decompress_io_ctx {
 #define MAX_COMPRESS_LOG_SIZE		8
 #define MAX_COMPRESS_WINDOW_SIZE	((PAGE_SIZE) << MAX_COMPRESS_LOG_SIZE)
 
+#define MAX_DUP_NAME			8
+#define MAX_SYSLINK_NAME		(MAX_VOLUME_NAME + MAX_DUP_NAME)
+#define MAX_DUP_NUM			100
+
 struct f2fs_sb_info {
 	struct super_block *sb;			/* pointer to VFS super block */
 	struct proc_dir_entry *s_proc;		/* proc entry */
@@ -1586,6 +1590,10 @@ struct f2fs_sb_info {
 
 	struct kmem_cache *inline_xattr_slab;	/* inline xattr entry */
 	unsigned int inline_xattr_slab_size;	/* default inline xattr slab size */
+
+	/* For sysfs symlink per volume */
+	char syslink_name[MAX_SYSLINK_NAME];
+	struct mutex syslink_mutex;
 };
 
 struct f2fs_private_dio {
@@ -3816,6 +3824,12 @@ int __init f2fs_init_sysfs(void);
 void f2fs_exit_sysfs(void);
 int f2fs_register_sysfs(struct f2fs_sb_info *sbi);
 void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi);
+void f2fs_reload_syslink(struct f2fs_sb_info *sbi);
+
+static inline bool f2fs_has_syslink(struct f2fs_sb_info *sbi)
+{
+	return strlen(sbi->syslink_name);
+}
 
 /* verity.c */
 extern const struct fsverity_operations f2fs_verityops;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 521987cd8772..4612f645007a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3418,6 +3418,8 @@ static int f2fs_ioc_setfslabel(struct file *filp, unsigned long arg)
 
 	up_write(&sbi->sb_lock);
 
+	f2fs_reload_syslink(sbi);
+
 	mnt_drop_write_file(filp);
 out:
 	kfree(vbuf);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2686b07ae7eb..7b002785417a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3021,6 +3021,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	init_rwsem(&sbi->sb_lock);
 	init_rwsem(&sbi->pin_sem);
+	mutex_init(&sbi->syslink_mutex);
 }
 
 static int init_percpu_info(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 50524401c8e6..e9818dd338c1 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -11,6 +11,7 @@
 #include <linux/f2fs_fs.h>
 #include <linux/seq_file.h>
 #include <linux/unicode.h>
+#include <linux/nls.h>
 
 #include "f2fs.h"
 #include "segment.h"
@@ -897,6 +898,51 @@ static int __maybe_unused victim_bits_seq_show(struct seq_file *seq,
 	return 0;
 }
 
+static void f2fs_unload_syslink(struct f2fs_sb_info *sbi)
+{
+	if (!f2fs_has_syslink(sbi))
+		return;
+
+	sysfs_remove_link(&f2fs_kset.kobj, sbi->syslink_name);
+	memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
+}
+
+static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
+{
+	int idx, count, ret;
+
+	down_read(&sbi->sb_lock);
+	count = utf16s_to_utf8s(sbi->raw_super->volume_name,
+			ARRAY_SIZE(sbi->raw_super->volume_name),
+			UTF16_LITTLE_ENDIAN, sbi->syslink_name,
+			MAX_VOLUME_NAME);
+	up_read(&sbi->sb_lock);
+
+	if (!count)
+		return -ENOENT;
+
+	for (idx = 0; idx < MAX_DUP_NUM; idx++) {
+		snprintf(sbi->syslink_name + count, MAX_DUP_NAME, "_%d", idx);
+		ret = sysfs_create_link(&f2fs_kset.kobj, &sbi->s_kobj,
+				sbi->syslink_name);
+		if (ret != -EEXIST)
+			break;
+	}
+
+	if (ret)
+		memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
+
+	return ret;
+}
+
+void f2fs_reload_syslink(struct f2fs_sb_info *sbi)
+{
+	mutex_lock(&sbi->syslink_mutex);
+	f2fs_unload_syslink(sbi);
+	f2fs_load_syslink(sbi);
+	mutex_unlock(&sbi->syslink_mutex);
+}
+
 int __init f2fs_init_sysfs(void)
 {
 	int ret;
@@ -954,11 +1000,15 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
 		proc_create_single_data("victim_bits", S_IRUGO, sbi->s_proc,
 				victim_bits_seq_show, sb);
 	}
+
+	f2fs_load_syslink(sbi);
+
 	return 0;
 }
 
 void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
 {
+	f2fs_unload_syslink(sbi);
 	if (sbi->s_proc) {
 		remove_proc_entry("iostat_info", sbi->s_proc);
 		remove_proc_entry("segment_info", sbi->s_proc);
-- 
2.28.0.rc0.105.gf9edc3c819-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] 16+ messages in thread

* [PATCH 2/2] f2fs: add volume_name mount option
  2020-07-19  5:44 ` [f2fs-dev] " Daeho Jeong
@ 2020-07-19  5:44   ` Daeho Jeong
  -1 siblings, 0 replies; 16+ messages in thread
From: Daeho Jeong @ 2020-07-19  5:44 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Added "volume_name" mount option. When the volume name in the on-disk
superblock doesn't exist, we can input the volume name as a mount
option and this is used to create a sysfs symbolic link pointing to
/sys/fs/f2fs/<disk>. The format of the symbolic directory link is like
/sys/fs/f2fs/<volume_name>_<num>, <volume_name> is the passed volume
name and <num> means the order of mounting with the same volume name.
When the on-disk volume name already exists, this mount option will be
ignored.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 Documentation/filesystems/f2fs.rst |  8 ++++++++
 fs/f2fs/super.c                    | 23 +++++++++++++++++++++++
 fs/f2fs/sysfs.c                    | 14 +++++++++++---
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 8221f3af6042..07507bed4fc1 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -260,6 +260,14 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
                        For other files, we can still enable compression via ioctl.
                        Note that, there is one reserved special extension '*', it
                        can be set to enable compression for all files.
+volume_name=%s         When the volume name in the on-disk superblock doesn't exist,
+                       we can input the volume name as a mount option and this is
+                       used to create a sysfs symbolic link pointing to
+                       /sys/fs/f2fs/<disk>. The format of the symbolic directory
+                       link is like /sys/fs/f2fs/<volume_name>_<num>, <volume_name>
+                       is the passed volume name and <num> means the order of mounting
+                       with the same volume name. When the on-disk volume name already
+                       exists, this mount option will be ignored.
 ====================== ============================================================
 
 Debugfs Entries
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7b002785417a..18d0a535697d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -145,6 +145,7 @@ enum {
 	Opt_compress_algorithm,
 	Opt_compress_log_size,
 	Opt_compress_extension,
+	Opt_volume_name,
 	Opt_err,
 };
 
@@ -211,6 +212,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_compress_algorithm, "compress_algorithm=%s"},
 	{Opt_compress_log_size, "compress_log_size=%u"},
 	{Opt_compress_extension, "compress_extension=%s"},
+	{Opt_volume_name, "volume_name=%s"},
 	{Opt_err, NULL},
 };
 
@@ -918,6 +920,21 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			F2FS_OPTION(sbi).compress_ext_cnt++;
 			kfree(name);
 			break;
+		case Opt_volume_name:
+			name = match_strdup(&args[0]);
+			if (!name)
+				return -ENOMEM;
+
+			if (strlen(name) > MAX_VOLUME_NAME) {
+				f2fs_err(sbi,
+					"Volume name is too long");
+				kfree(name);
+				return -EINVAL;
+			}
+
+			strncpy(sbi->syslink_name, name, MAX_VOLUME_NAME);
+			kfree(name);
+			break;
 		default:
 			f2fs_err(sbi, "Unrecognized mount option \"%s\" or missing value",
 				 p);
@@ -1609,6 +1626,12 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_printf(seq, ",fsync_mode=%s", "nobarrier");
 
 	f2fs_show_compress_options(seq, sbi->sb);
+
+	mutex_lock(&sbi->syslink_mutex);
+	if (f2fs_has_syslink(sbi))
+		seq_printf(seq, ",volume_name=%s", sbi->syslink_name);
+	mutex_unlock(&sbi->syslink_mutex);
+
 	return 0;
 }
 
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index e9818dd338c1..6d4a2f8aa0d7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -907,7 +907,7 @@ static void f2fs_unload_syslink(struct f2fs_sb_info *sbi)
 	memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
 }
 
-static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
+static int f2fs_load_syslink(struct f2fs_sb_info *sbi, bool mount)
 {
 	int idx, count, ret;
 
@@ -918,6 +918,14 @@ static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
 			MAX_VOLUME_NAME);
 	up_read(&sbi->sb_lock);
 
+	if (mount) {
+		if (count)
+			memset(sbi->syslink_name + count, 0,
+					MAX_SYSLINK_NAME - count);
+		else
+			count = strlen(sbi->syslink_name);
+	}
+
 	if (!count)
 		return -ENOENT;
 
@@ -939,7 +947,7 @@ void f2fs_reload_syslink(struct f2fs_sb_info *sbi)
 {
 	mutex_lock(&sbi->syslink_mutex);
 	f2fs_unload_syslink(sbi);
-	f2fs_load_syslink(sbi);
+	f2fs_load_syslink(sbi, false);
 	mutex_unlock(&sbi->syslink_mutex);
 }
 
@@ -1001,7 +1009,7 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
 				victim_bits_seq_show, sb);
 	}
 
-	f2fs_load_syslink(sbi);
+	f2fs_load_syslink(sbi, true);
 
 	return 0;
 }
-- 
2.28.0.rc0.105.gf9edc3c819-goog


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

* [f2fs-dev] [PATCH 2/2] f2fs: add volume_name mount option
@ 2020-07-19  5:44   ` Daeho Jeong
  0 siblings, 0 replies; 16+ messages in thread
From: Daeho Jeong @ 2020-07-19  5:44 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Added "volume_name" mount option. When the volume name in the on-disk
superblock doesn't exist, we can input the volume name as a mount
option and this is used to create a sysfs symbolic link pointing to
/sys/fs/f2fs/<disk>. The format of the symbolic directory link is like
/sys/fs/f2fs/<volume_name>_<num>, <volume_name> is the passed volume
name and <num> means the order of mounting with the same volume name.
When the on-disk volume name already exists, this mount option will be
ignored.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 Documentation/filesystems/f2fs.rst |  8 ++++++++
 fs/f2fs/super.c                    | 23 +++++++++++++++++++++++
 fs/f2fs/sysfs.c                    | 14 +++++++++++---
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 8221f3af6042..07507bed4fc1 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -260,6 +260,14 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
                        For other files, we can still enable compression via ioctl.
                        Note that, there is one reserved special extension '*', it
                        can be set to enable compression for all files.
+volume_name=%s         When the volume name in the on-disk superblock doesn't exist,
+                       we can input the volume name as a mount option and this is
+                       used to create a sysfs symbolic link pointing to
+                       /sys/fs/f2fs/<disk>. The format of the symbolic directory
+                       link is like /sys/fs/f2fs/<volume_name>_<num>, <volume_name>
+                       is the passed volume name and <num> means the order of mounting
+                       with the same volume name. When the on-disk volume name already
+                       exists, this mount option will be ignored.
 ====================== ============================================================
 
 Debugfs Entries
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7b002785417a..18d0a535697d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -145,6 +145,7 @@ enum {
 	Opt_compress_algorithm,
 	Opt_compress_log_size,
 	Opt_compress_extension,
+	Opt_volume_name,
 	Opt_err,
 };
 
@@ -211,6 +212,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_compress_algorithm, "compress_algorithm=%s"},
 	{Opt_compress_log_size, "compress_log_size=%u"},
 	{Opt_compress_extension, "compress_extension=%s"},
+	{Opt_volume_name, "volume_name=%s"},
 	{Opt_err, NULL},
 };
 
@@ -918,6 +920,21 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			F2FS_OPTION(sbi).compress_ext_cnt++;
 			kfree(name);
 			break;
+		case Opt_volume_name:
+			name = match_strdup(&args[0]);
+			if (!name)
+				return -ENOMEM;
+
+			if (strlen(name) > MAX_VOLUME_NAME) {
+				f2fs_err(sbi,
+					"Volume name is too long");
+				kfree(name);
+				return -EINVAL;
+			}
+
+			strncpy(sbi->syslink_name, name, MAX_VOLUME_NAME);
+			kfree(name);
+			break;
 		default:
 			f2fs_err(sbi, "Unrecognized mount option \"%s\" or missing value",
 				 p);
@@ -1609,6 +1626,12 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_printf(seq, ",fsync_mode=%s", "nobarrier");
 
 	f2fs_show_compress_options(seq, sbi->sb);
+
+	mutex_lock(&sbi->syslink_mutex);
+	if (f2fs_has_syslink(sbi))
+		seq_printf(seq, ",volume_name=%s", sbi->syslink_name);
+	mutex_unlock(&sbi->syslink_mutex);
+
 	return 0;
 }
 
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index e9818dd338c1..6d4a2f8aa0d7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -907,7 +907,7 @@ static void f2fs_unload_syslink(struct f2fs_sb_info *sbi)
 	memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
 }
 
-static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
+static int f2fs_load_syslink(struct f2fs_sb_info *sbi, bool mount)
 {
 	int idx, count, ret;
 
@@ -918,6 +918,14 @@ static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
 			MAX_VOLUME_NAME);
 	up_read(&sbi->sb_lock);
 
+	if (mount) {
+		if (count)
+			memset(sbi->syslink_name + count, 0,
+					MAX_SYSLINK_NAME - count);
+		else
+			count = strlen(sbi->syslink_name);
+	}
+
 	if (!count)
 		return -ENOENT;
 
@@ -939,7 +947,7 @@ void f2fs_reload_syslink(struct f2fs_sb_info *sbi)
 {
 	mutex_lock(&sbi->syslink_mutex);
 	f2fs_unload_syslink(sbi);
-	f2fs_load_syslink(sbi);
+	f2fs_load_syslink(sbi, false);
 	mutex_unlock(&sbi->syslink_mutex);
 }
 
@@ -1001,7 +1009,7 @@ int f2fs_register_sysfs(struct f2fs_sb_info *sbi)
 				victim_bits_seq_show, sb);
 	}
 
-	f2fs_load_syslink(sbi);
+	f2fs_load_syslink(sbi, true);
 
 	return 0;
 }
-- 
2.28.0.rc0.105.gf9edc3c819-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] 16+ messages in thread

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-19  5:44 ` [f2fs-dev] " Daeho Jeong
@ 2020-07-19  9:06   ` kernel test robot
  -1 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-07-19  9:06 UTC (permalink / raw)
  To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team
  Cc: kbuild-all, Daeho Jeong

[-- Attachment #1: Type: text/plain, Size: 2572 bytes --]

Hi Daeho,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on linux/master linus/master v5.8-rc5 next-20200717]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-sysfs-symbolic-link-to-kobject-with-volume-name/20200719-134628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: s390-randconfig-s032-20200719 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-49-g707c5017-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> fs/f2fs/sysfs.c:915:36: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short const [usertype] *pwcs @@     got restricted __le16 * @@
>> fs/f2fs/sysfs.c:915:36: sparse:     expected unsigned short const [usertype] *pwcs
>> fs/f2fs/sysfs.c:915:36: sparse:     got restricted __le16 *

vim +915 fs/f2fs/sysfs.c

   909	
   910	static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
   911	{
   912		int idx, count, ret;
   913	
   914		down_read(&sbi->sb_lock);
 > 915		count = utf16s_to_utf8s(sbi->raw_super->volume_name,
   916				ARRAY_SIZE(sbi->raw_super->volume_name),
   917				UTF16_LITTLE_ENDIAN, sbi->syslink_name,
   918				MAX_VOLUME_NAME);
   919		up_read(&sbi->sb_lock);
   920	
   921		if (!count)
   922			return -ENOENT;
   923	
   924		for (idx = 0; idx < MAX_DUP_NUM; idx++) {
   925			snprintf(sbi->syslink_name + count, MAX_DUP_NAME, "_%d", idx);
   926			ret = sysfs_create_link(&f2fs_kset.kobj, &sbi->s_kobj,
   927					sbi->syslink_name);
   928			if (ret != -EEXIST)
   929				break;
   930		}
   931	
   932		if (ret)
   933			memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
   934	
   935		return ret;
   936	}
   937	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27356 bytes --]

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

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-19  9:06   ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-07-19  9:06 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

Hi Daeho,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on linux/master linus/master v5.8-rc5 next-20200717]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-sysfs-symbolic-link-to-kobject-with-volume-name/20200719-134628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: s390-randconfig-s032-20200719 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-49-g707c5017-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> fs/f2fs/sysfs.c:915:36: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short const [usertype] *pwcs @@     got restricted __le16 * @@
>> fs/f2fs/sysfs.c:915:36: sparse:     expected unsigned short const [usertype] *pwcs
>> fs/f2fs/sysfs.c:915:36: sparse:     got restricted __le16 *

vim +915 fs/f2fs/sysfs.c

   909	
   910	static int f2fs_load_syslink(struct f2fs_sb_info *sbi)
   911	{
   912		int idx, count, ret;
   913	
   914		down_read(&sbi->sb_lock);
 > 915		count = utf16s_to_utf8s(sbi->raw_super->volume_name,
   916				ARRAY_SIZE(sbi->raw_super->volume_name),
   917				UTF16_LITTLE_ENDIAN, sbi->syslink_name,
   918				MAX_VOLUME_NAME);
   919		up_read(&sbi->sb_lock);
   920	
   921		if (!count)
   922			return -ENOENT;
   923	
   924		for (idx = 0; idx < MAX_DUP_NUM; idx++) {
   925			snprintf(sbi->syslink_name + count, MAX_DUP_NAME, "_%d", idx);
   926			ret = sysfs_create_link(&f2fs_kset.kobj, &sbi->s_kobj,
   927					sbi->syslink_name);
   928			if (ret != -EEXIST)
   929				break;
   930		}
   931	
   932		if (ret)
   933			memset(sbi->syslink_name, 0, MAX_SYSLINK_NAME);
   934	
   935		return ret;
   936	}
   937	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 27356 bytes --]

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

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-19  5:44 ` [f2fs-dev] " Daeho Jeong
@ 2020-07-19 15:16   ` Greg KH
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-19 15:16 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> Added a symbolic link directory pointing to its device name
> directory using the volume name of the partition in sysfs.
> (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)

No, please no.

That is already created today for you in /dev/disk/  The kernel does not
need to do this again.

If your distro/system/whatever does not provide you with /dev/disk/ and
all of the symlinks in there, then work with your distro/system/whatever
to do so.

Again, no need to do this on a per-filesystem-basis when we already have
this around for all filesystems, and have had it for 15+ years now.

thanks,

greg k-h

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

* Re: [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-19 15:16   ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-19 15:16 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> Added a symbolic link directory pointing to its device name
> directory using the volume name of the partition in sysfs.
> (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)

No, please no.

That is already created today for you in /dev/disk/  The kernel does not
need to do this again.

If your distro/system/whatever does not provide you with /dev/disk/ and
all of the symlinks in there, then work with your distro/system/whatever
to do so.

Again, no need to do this on a per-filesystem-basis when we already have
this around for all filesystems, and have had it for 15+ years now.

thanks,

greg k-h


_______________________________________________
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] 16+ messages in thread

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-19 15:16   ` [f2fs-dev] " Greg KH
@ 2020-07-22 16:43     ` Jaegeuk Kim
  -1 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2020-07-22 16:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On 07/19, Greg KH wrote:
> On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> > 
> > Added a symbolic link directory pointing to its device name
> > directory using the volume name of the partition in sysfs.
> > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> 
> No, please no.
> 
> That is already created today for you in /dev/disk/  The kernel does not
> need to do this again.
> 
> If your distro/system/whatever does not provide you with /dev/disk/ and
> all of the symlinks in there, then work with your distro/system/whatever
> to do so.

I don't get the point, since /dev/disk points device node, not any sysfs entry.
Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?

> 
> Again, no need to do this on a per-filesystem-basis when we already have
> this around for all filesystems, and have had it for 15+ years now.

Could you point out where we can get this? And, the label support depends
on per-filesystem design. I'm not sure how this can be generic enough.

> 
> thanks,
> 
> greg k-h

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

* Re: [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-22 16:43     ` Jaegeuk Kim
  0 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2020-07-22 16:43 UTC (permalink / raw)
  To: Greg KH; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On 07/19, Greg KH wrote:
> On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> > 
> > Added a symbolic link directory pointing to its device name
> > directory using the volume name of the partition in sysfs.
> > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> 
> No, please no.
> 
> That is already created today for you in /dev/disk/  The kernel does not
> need to do this again.
> 
> If your distro/system/whatever does not provide you with /dev/disk/ and
> all of the symlinks in there, then work with your distro/system/whatever
> to do so.

I don't get the point, since /dev/disk points device node, not any sysfs entry.
Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?

> 
> Again, no need to do this on a per-filesystem-basis when we already have
> this around for all filesystems, and have had it for 15+ years now.

Could you point out where we can get this? And, the label support depends
on per-filesystem design. I'm not sure how this can be generic enough.

> 
> thanks,
> 
> greg k-h


_______________________________________________
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] 16+ messages in thread

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-22 16:43     ` [f2fs-dev] " Jaegeuk Kim
@ 2020-07-22 17:06       ` Greg KH
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-22 17:06 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> On 07/19, Greg KH wrote:
> > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > From: Daeho Jeong <daehojeong@google.com>
> > > 
> > > Added a symbolic link directory pointing to its device name
> > > directory using the volume name of the partition in sysfs.
> > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > 
> > No, please no.
> > 
> > That is already created today for you in /dev/disk/  The kernel does not
> > need to do this again.
> > 
> > If your distro/system/whatever does not provide you with /dev/disk/ and
> > all of the symlinks in there, then work with your distro/system/whatever
> > to do so.
> 
> I don't get the point, since /dev/disk points device node, not any sysfs entry.
> Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?

Huh, no!  It's all done for you today automagically by userspace:

$ tree /dev/disk/by-label/
/dev/disk/by-label/
├── boot -> ../../sda1
├── fast_disk -> ../../md0
├── root -> ../../sda2
└── stuff -> ../../dm-0

Look on your laptop/desktop/server today for those, there's lots of
symlinks in /dev/disk/

> > Again, no need to do this on a per-filesystem-basis when we already have
> > this around for all filesystems, and have had it for 15+ years now.
> 
> Could you point out where we can get this? And, the label support depends
> on per-filesystem design. I'm not sure how this can be generic enough.

Userspace knows how to read labels on a per-filesystem-basis and does so
just fine.  That's how it creates those symlinks, no kernel support is
needed.

This has been implemented for 15+ years now, it's not a new thing...

Now if your embedded system doesn't support it, that's the userspace of
that system's fault, it's not the kernel's fault at all.  Go fix your
userspace if you want those things.

thanks,

greg k-h

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

* Re: [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-22 17:06       ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-22 17:06 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> On 07/19, Greg KH wrote:
> > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > From: Daeho Jeong <daehojeong@google.com>
> > > 
> > > Added a symbolic link directory pointing to its device name
> > > directory using the volume name of the partition in sysfs.
> > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > 
> > No, please no.
> > 
> > That is already created today for you in /dev/disk/  The kernel does not
> > need to do this again.
> > 
> > If your distro/system/whatever does not provide you with /dev/disk/ and
> > all of the symlinks in there, then work with your distro/system/whatever
> > to do so.
> 
> I don't get the point, since /dev/disk points device node, not any sysfs entry.
> Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?

Huh, no!  It's all done for you today automagically by userspace:

$ tree /dev/disk/by-label/
/dev/disk/by-label/
├── boot -> ../../sda1
├── fast_disk -> ../../md0
├── root -> ../../sda2
└── stuff -> ../../dm-0

Look on your laptop/desktop/server today for those, there's lots of
symlinks in /dev/disk/

> > Again, no need to do this on a per-filesystem-basis when we already have
> > this around for all filesystems, and have had it for 15+ years now.
> 
> Could you point out where we can get this? And, the label support depends
> on per-filesystem design. I'm not sure how this can be generic enough.

Userspace knows how to read labels on a per-filesystem-basis and does so
just fine.  That's how it creates those symlinks, no kernel support is
needed.

This has been implemented for 15+ years now, it's not a new thing...

Now if your embedded system doesn't support it, that's the userspace of
that system's fault, it's not the kernel's fault at all.  Go fix your
userspace if you want those things.

thanks,

greg k-h


_______________________________________________
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] 16+ messages in thread

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-22 17:06       ` [f2fs-dev] " Greg KH
@ 2020-07-22 17:24         ` Jaegeuk Kim
  -1 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2020-07-22 17:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On 07/22, Greg KH wrote:
> On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> > On 07/19, Greg KH wrote:
> > > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > > From: Daeho Jeong <daehojeong@google.com>
> > > > 
> > > > Added a symbolic link directory pointing to its device name
> > > > directory using the volume name of the partition in sysfs.
> > > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > > 
> > > No, please no.
> > > 
> > > That is already created today for you in /dev/disk/  The kernel does not
> > > need to do this again.
> > > 
> > > If your distro/system/whatever does not provide you with /dev/disk/ and
> > > all of the symlinks in there, then work with your distro/system/whatever
> > > to do so.
> > 
> > I don't get the point, since /dev/disk points device node, not any sysfs entry.
> > Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?
> 
> Huh, no!  It's all done for you today automagically by userspace:
> 
> $ tree /dev/disk/by-label/
> /dev/disk/by-label/
> ├── boot -> ../../sda1
> ├── fast_disk -> ../../md0
> ├── root -> ../../sda2
> └── stuff -> ../../dm-0
> 
> Look on your laptop/desktop/server today for those, there's lots of
> symlinks in /dev/disk/

What I mean is "creating symlink from *userspace*", but the concern is
"/dev/" looks like being used for device nodes only, not sysfs.

> 
> > > Again, no need to do this on a per-filesystem-basis when we already have
> > > this around for all filesystems, and have had it for 15+ years now.
> > 
> > Could you point out where we can get this? And, the label support depends
> > on per-filesystem design. I'm not sure how this can be generic enough.
> 
> Userspace knows how to read labels on a per-filesystem-basis and does so
> just fine.  That's how it creates those symlinks, no kernel support is
> needed.
> 
> This has been implemented for 15+ years now, it's not a new thing...
> 
> Now if your embedded system doesn't support it, that's the userspace of
> that system's fault, it's not the kernel's fault at all.  Go fix your
> userspace if you want those things.

I'm not talking about whose fault tho. :) By any chance, could you please
suggest a good location to create a symlink for this sysfs entry?

> 
> thanks,
> 
> greg k-h

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

* Re: [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-22 17:24         ` Jaegeuk Kim
  0 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2020-07-22 17:24 UTC (permalink / raw)
  To: Greg KH; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On 07/22, Greg KH wrote:
> On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> > On 07/19, Greg KH wrote:
> > > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > > From: Daeho Jeong <daehojeong@google.com>
> > > > 
> > > > Added a symbolic link directory pointing to its device name
> > > > directory using the volume name of the partition in sysfs.
> > > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > > 
> > > No, please no.
> > > 
> > > That is already created today for you in /dev/disk/  The kernel does not
> > > need to do this again.
> > > 
> > > If your distro/system/whatever does not provide you with /dev/disk/ and
> > > all of the symlinks in there, then work with your distro/system/whatever
> > > to do so.
> > 
> > I don't get the point, since /dev/disk points device node, not any sysfs entry.
> > Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?
> 
> Huh, no!  It's all done for you today automagically by userspace:
> 
> $ tree /dev/disk/by-label/
> /dev/disk/by-label/
> ├── boot -> ../../sda1
> ├── fast_disk -> ../../md0
> ├── root -> ../../sda2
> └── stuff -> ../../dm-0
> 
> Look on your laptop/desktop/server today for those, there's lots of
> symlinks in /dev/disk/

What I mean is "creating symlink from *userspace*", but the concern is
"/dev/" looks like being used for device nodes only, not sysfs.

> 
> > > Again, no need to do this on a per-filesystem-basis when we already have
> > > this around for all filesystems, and have had it for 15+ years now.
> > 
> > Could you point out where we can get this? And, the label support depends
> > on per-filesystem design. I'm not sure how this can be generic enough.
> 
> Userspace knows how to read labels on a per-filesystem-basis and does so
> just fine.  That's how it creates those symlinks, no kernel support is
> needed.
> 
> This has been implemented for 15+ years now, it's not a new thing...
> 
> Now if your embedded system doesn't support it, that's the userspace of
> that system's fault, it's not the kernel's fault at all.  Go fix your
> userspace if you want those things.

I'm not talking about whose fault tho. :) By any chance, could you please
suggest a good location to create a symlink for this sysfs entry?

> 
> thanks,
> 
> greg k-h


_______________________________________________
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] 16+ messages in thread

* Re: [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
  2020-07-22 17:24         ` [f2fs-dev] " Jaegeuk Kim
@ 2020-07-22 19:10           ` Greg KH
  -1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-22 19:10 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On Wed, Jul 22, 2020 at 10:24:37AM -0700, Jaegeuk Kim wrote:
> On 07/22, Greg KH wrote:
> > On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> > > On 07/19, Greg KH wrote:
> > > > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > > > From: Daeho Jeong <daehojeong@google.com>
> > > > > 
> > > > > Added a symbolic link directory pointing to its device name
> > > > > directory using the volume name of the partition in sysfs.
> > > > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > > > 
> > > > No, please no.
> > > > 
> > > > That is already created today for you in /dev/disk/  The kernel does not
> > > > need to do this again.
> > > > 
> > > > If your distro/system/whatever does not provide you with /dev/disk/ and
> > > > all of the symlinks in there, then work with your distro/system/whatever
> > > > to do so.
> > > 
> > > I don't get the point, since /dev/disk points device node, not any sysfs entry.
> > > Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?
> > 
> > Huh, no!  It's all done for you today automagically by userspace:
> > 
> > $ tree /dev/disk/by-label/
> > /dev/disk/by-label/
> > ├── boot -> ../../sda1
> > ├── fast_disk -> ../../md0
> > ├── root -> ../../sda2
> > └── stuff -> ../../dm-0
> > 
> > Look on your laptop/desktop/server today for those, there's lots of
> > symlinks in /dev/disk/
> 
> What I mean is "creating symlink from *userspace*", but the concern is
> "/dev/" looks like being used for device nodes only, not sysfs.

That is correct, that is what /dev/ is for, not sysfs.

> > > > Again, no need to do this on a per-filesystem-basis when we already have
> > > > this around for all filesystems, and have had it for 15+ years now.
> > > 
> > > Could you point out where we can get this? And, the label support depends
> > > on per-filesystem design. I'm not sure how this can be generic enough.
> > 
> > Userspace knows how to read labels on a per-filesystem-basis and does so
> > just fine.  That's how it creates those symlinks, no kernel support is
> > needed.
> > 
> > This has been implemented for 15+ years now, it's not a new thing...
> > 
> > Now if your embedded system doesn't support it, that's the userspace of
> > that system's fault, it's not the kernel's fault at all.  Go fix your
> > userspace if you want those things.
> 
> I'm not talking about whose fault tho. :) By any chance, could you please
> suggest a good location to create a symlink for this sysfs entry?

There is no need for such a sysfs entry, that's what I am trying to say.
Userspace already has all of the needed information here, do not try to
add filesystem-specific stuff like this, unless you somehow are going to
do it for all filesystems :)

thanks,

gregt k-h

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

* Re: [f2fs-dev] [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name
@ 2020-07-22 19:10           ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-07-22 19:10 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On Wed, Jul 22, 2020 at 10:24:37AM -0700, Jaegeuk Kim wrote:
> On 07/22, Greg KH wrote:
> > On Wed, Jul 22, 2020 at 09:43:56AM -0700, Jaegeuk Kim wrote:
> > > On 07/19, Greg KH wrote:
> > > > On Sun, Jul 19, 2020 at 02:44:08PM +0900, Daeho Jeong wrote:
> > > > > From: Daeho Jeong <daehojeong@google.com>
> > > > > 
> > > > > Added a symbolic link directory pointing to its device name
> > > > > directory using the volume name of the partition in sysfs.
> > > > > (i.e., /sys/fs/f2fs/vol_#x -> /sys/fs/f2fs/sda1)
> > > > 
> > > > No, please no.
> > > > 
> > > > That is already created today for you in /dev/disk/  The kernel does not
> > > > need to do this again.
> > > > 
> > > > If your distro/system/whatever does not provide you with /dev/disk/ and
> > > > all of the symlinks in there, then work with your distro/system/whatever
> > > > to do so.
> > > 
> > > I don't get the point, since /dev/disk points device node, not any sysfs entry.
> > > Do you mean we need to create symlink to /sys/fs/f2fs/dm-X in /dev/disk?
> > 
> > Huh, no!  It's all done for you today automagically by userspace:
> > 
> > $ tree /dev/disk/by-label/
> > /dev/disk/by-label/
> > ├── boot -> ../../sda1
> > ├── fast_disk -> ../../md0
> > ├── root -> ../../sda2
> > └── stuff -> ../../dm-0
> > 
> > Look on your laptop/desktop/server today for those, there's lots of
> > symlinks in /dev/disk/
> 
> What I mean is "creating symlink from *userspace*", but the concern is
> "/dev/" looks like being used for device nodes only, not sysfs.

That is correct, that is what /dev/ is for, not sysfs.

> > > > Again, no need to do this on a per-filesystem-basis when we already have
> > > > this around for all filesystems, and have had it for 15+ years now.
> > > 
> > > Could you point out where we can get this? And, the label support depends
> > > on per-filesystem design. I'm not sure how this can be generic enough.
> > 
> > Userspace knows how to read labels on a per-filesystem-basis and does so
> > just fine.  That's how it creates those symlinks, no kernel support is
> > needed.
> > 
> > This has been implemented for 15+ years now, it's not a new thing...
> > 
> > Now if your embedded system doesn't support it, that's the userspace of
> > that system's fault, it's not the kernel's fault at all.  Go fix your
> > userspace if you want those things.
> 
> I'm not talking about whose fault tho. :) By any chance, could you please
> suggest a good location to create a symlink for this sysfs entry?

There is no need for such a sysfs entry, that's what I am trying to say.
Userspace already has all of the needed information here, do not try to
add filesystem-specific stuff like this, unless you somehow are going to
do it for all filesystems :)

thanks,

gregt k-h


_______________________________________________
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] 16+ messages in thread

end of thread, other threads:[~2020-07-22 19:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-19  5:44 [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name Daeho Jeong
2020-07-19  5:44 ` [f2fs-dev] " Daeho Jeong
2020-07-19  5:44 ` [PATCH 2/2] f2fs: add volume_name mount option Daeho Jeong
2020-07-19  5:44   ` [f2fs-dev] " Daeho Jeong
2020-07-19  9:06 ` [PATCH 1/2] f2fs: add sysfs symbolic link to kobject with volume name kernel test robot
2020-07-19  9:06   ` kernel test robot
2020-07-19 15:16 ` Greg KH
2020-07-19 15:16   ` [f2fs-dev] " Greg KH
2020-07-22 16:43   ` Jaegeuk Kim
2020-07-22 16:43     ` [f2fs-dev] " Jaegeuk Kim
2020-07-22 17:06     ` Greg KH
2020-07-22 17:06       ` [f2fs-dev] " Greg KH
2020-07-22 17:24       ` Jaegeuk Kim
2020-07-22 17:24         ` [f2fs-dev] " Jaegeuk Kim
2020-07-22 19:10         ` Greg KH
2020-07-22 19:10           ` [f2fs-dev] " Greg KH

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.