[PATCH] ubifs: Convert ubifs to use the new mount API

[PATCH] ubifs: Convert ubifs to use the new mount API

[Eric Sandeen]

From: David Howells <dhowells@redhat.com>

Convert the ubifs filesystem to the new internal mount API as the old
one will be obsoleted and removed.  This allows greater flexibility in
communication of mount parameters between userspace, the VFS and the
filesystem.

See Documentation/filesystems/mount_api.txt for more information.

Signed-off-by: David Howells <dhowells@redhat.com>
[sandeen: forward-port old patch]
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
cc: Richard Weinberger <richard@nod.at>
cc: Zhihao Cheng <chengzhihao1@huawei.com>
cc: linux-mtd@lists.infradead.org
---
 fs/ubifs/super.c | 459 +++++++++++++++++++++--------------------------
 1 file changed, 204 insertions(+), 255 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 291583005dd1..cf2e9104baff 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -19,9 +19,9 @@
 #include <linux/module.h>
 #include <linux/ctype.h>
 #include <linux/kthread.h>
-#include <linux/parser.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/seq_file.h>
-#include <linux/mount.h>
 #include <linux/math64.h>
 #include <linux/writeback.h>
 #include "ubifs.h"
@@ -963,7 +963,7 @@ static int check_volume_empty(struct ubifs_info *c)
  * Opt_no_bulk_read: disable bulk-reads
  * Opt_chk_data_crc: check CRCs when reading data nodes
  * Opt_no_chk_data_crc: do not check CRCs when reading data nodes
- * Opt_override_compr: override default compressor
+ * Opt_compr: override default compressor
  * Opt_assert: set ubifs_assert() action
  * Opt_auth_key: The key name used for authentication
  * Opt_auth_hash_name: The hash type used for authentication
@@ -976,54 +976,46 @@ enum {
 	Opt_no_bulk_read,
 	Opt_chk_data_crc,
 	Opt_no_chk_data_crc,
-	Opt_override_compr,
+	Opt_compr,
 	Opt_assert,
 	Opt_auth_key,
 	Opt_auth_hash_name,
 	Opt_ignore,
-	Opt_err,
 };
 
-static const match_table_t tokens = {
-	{Opt_fast_unmount, "fast_unmount"},
-	{Opt_norm_unmount, "norm_unmount"},
-	{Opt_bulk_read, "bulk_read"},
-	{Opt_no_bulk_read, "no_bulk_read"},
-	{Opt_chk_data_crc, "chk_data_crc"},
-	{Opt_no_chk_data_crc, "no_chk_data_crc"},
-	{Opt_override_compr, "compr=%s"},
-	{Opt_auth_key, "auth_key=%s"},
-	{Opt_auth_hash_name, "auth_hash_name=%s"},
-	{Opt_ignore, "ubi=%s"},
-	{Opt_ignore, "vol=%s"},
-	{Opt_assert, "assert=%s"},
-	{Opt_err, NULL},
+static const struct constant_table ubifs_param_compr[] = {
+	{ "none",		UBIFS_COMPR_NONE },
+	{ "lzo",		UBIFS_COMPR_LZO },
+	{ "zlib",		UBIFS_COMPR_ZLIB },
+	{ "zstd",		UBIFS_COMPR_ZSTD },
+	{}
 };
 
-/**
- * parse_standard_option - parse a standard mount option.
- * @option: the option to parse
- *
- * Normally, standard mount options like "sync" are passed to file-systems as
- * flags. However, when a "rootflags=" kernel boot parameter is used, they may
- * be present in the options string. This function tries to deal with this
- * situation and parse standard options. Returns 0 if the option was not
- * recognized, and the corresponding integer flag if it was.
- *
- * UBIFS is only interested in the "sync" option, so do not check for anything
- * else.
- */
-static int parse_standard_option(const char *option)
-{
+static const struct constant_table ubifs_param_assert[] = {
+	{ "report",	ASSACT_REPORT },
+	{ "read-only",	ASSACT_RO },
+	{ "panic",	ASSACT_PANIC },
+	{}
+};
 
-	pr_notice("UBIFS: parse %s\n", option);
-	if (!strcmp(option, "sync"))
-		return SB_SYNCHRONOUS;
-	return 0;
-}
+static const struct fs_parameter_spec ubifs_fs_param_spec[] = {
+	fsparam_flag	("fast_unmount",		Opt_fast_unmount),
+	fsparam_flag	("norm_unmount",		Opt_norm_unmount),
+	fsparam_flag	("bulk_read",			Opt_bulk_read),
+	fsparam_flag	("no_bulk_read",		Opt_no_bulk_read),
+	fsparam_flag	("chk_data_crc",		Opt_chk_data_crc),
+	fsparam_flag	("no_chk_data_crc",		Opt_no_chk_data_crc),
+	fsparam_enum	("compr",			Opt_compr, ubifs_param_compr),
+	fsparam_enum	("assert",			Opt_assert, ubifs_param_assert),
+	fsparam_string	("auth_key",			Opt_auth_key),
+	fsparam_string	("auth_hash_name",		Opt_auth_hash_name),
+	fsparam_string	("ubi",				Opt_ignore),
+	fsparam_string	("vol",				Opt_ignore),
+	{}
+};
 
 /**
- * ubifs_parse_options - parse mount parameters.
+ * ubifs_parse_param - parse a parameter.
  * @c: UBIFS file-system description object
  * @options: parameters to parse
  * @is_remount: non-zero if this is FS re-mount
@@ -1031,127 +1023,69 @@ static int parse_standard_option(const char *option)
  * This function parses UBIFS mount options and returns zero in case success
  * and a negative error code in case of failure.
  */
-static int ubifs_parse_options(struct ubifs_info *c, char *options,
-			       int is_remount)
+static int ubifs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-
-	if (!options)
-		return 0;
-
-	while ((p = strsep(&options, ","))) {
-		int token;
+	struct ubifs_info *c = fc->s_fs_info;
+	struct fs_parse_result result;
+	bool is_remount = (fc->purpose & FS_CONTEXT_FOR_RECONFIGURE);
+	int opt;
 
-		if (!*p)
-			continue;
+	opt = fs_parse(fc, ubifs_fs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
 
-		token = match_token(p, tokens, args);
-		switch (token) {
+	switch (opt) {
 		/*
 		 * %Opt_fast_unmount and %Opt_norm_unmount options are ignored.
 		 * We accept them in order to be backward-compatible. But this
 		 * should be removed at some point.
 		 */
-		case Opt_fast_unmount:
-			c->mount_opts.unmount_mode = 2;
-			break;
-		case Opt_norm_unmount:
-			c->mount_opts.unmount_mode = 1;
-			break;
-		case Opt_bulk_read:
-			c->mount_opts.bulk_read = 2;
-			c->bulk_read = 1;
-			break;
-		case Opt_no_bulk_read:
-			c->mount_opts.bulk_read = 1;
-			c->bulk_read = 0;
-			break;
-		case Opt_chk_data_crc:
-			c->mount_opts.chk_data_crc = 2;
-			c->no_chk_data_crc = 0;
-			break;
-		case Opt_no_chk_data_crc:
-			c->mount_opts.chk_data_crc = 1;
-			c->no_chk_data_crc = 1;
-			break;
-		case Opt_override_compr:
-		{
-			char *name = match_strdup(&args[0]);
-
-			if (!name)
-				return -ENOMEM;
-			if (!strcmp(name, "none"))
-				c->mount_opts.compr_type = UBIFS_COMPR_NONE;
-			else if (!strcmp(name, "lzo"))
-				c->mount_opts.compr_type = UBIFS_COMPR_LZO;
-			else if (!strcmp(name, "zlib"))
-				c->mount_opts.compr_type = UBIFS_COMPR_ZLIB;
-			else if (!strcmp(name, "zstd"))
-				c->mount_opts.compr_type = UBIFS_COMPR_ZSTD;
-			else {
-				ubifs_err(c, "unknown compressor \"%s\"", name); //FIXME: is c ready?
-				kfree(name);
-				return -EINVAL;
-			}
-			kfree(name);
-			c->mount_opts.override_compr = 1;
-			c->default_compr = c->mount_opts.compr_type;
-			break;
-		}
-		case Opt_assert:
-		{
-			char *act = match_strdup(&args[0]);
-
-			if (!act)
-				return -ENOMEM;
-			if (!strcmp(act, "report"))
-				c->assert_action = ASSACT_REPORT;
-			else if (!strcmp(act, "read-only"))
-				c->assert_action = ASSACT_RO;
-			else if (!strcmp(act, "panic"))
-				c->assert_action = ASSACT_PANIC;
-			else {
-				ubifs_err(c, "unknown assert action \"%s\"", act);
-				kfree(act);
-				return -EINVAL;
-			}
-			kfree(act);
-			break;
-		}
-		case Opt_auth_key:
-			if (!is_remount) {
-				c->auth_key_name = kstrdup(args[0].from,
-								GFP_KERNEL);
-				if (!c->auth_key_name)
-					return -ENOMEM;
-			}
-			break;
-		case Opt_auth_hash_name:
-			if (!is_remount) {
-				c->auth_hash_name = kstrdup(args[0].from,
-								GFP_KERNEL);
-				if (!c->auth_hash_name)
-					return -ENOMEM;
-			}
-			break;
-		case Opt_ignore:
-			break;
-		default:
-		{
-			unsigned long flag;
-			struct super_block *sb = c->vfs_sb;
-
-			flag = parse_standard_option(p);
-			if (!flag) {
-				ubifs_err(c, "unrecognized mount option \"%s\" or missing value",
-					  p);
-				return -EINVAL;
-			}
-			sb->s_flags |= flag;
-			break;
+	case Opt_fast_unmount:
+		c->mount_opts.unmount_mode = 2;
+		break;
+	case Opt_norm_unmount:
+		c->mount_opts.unmount_mode = 1;
+		break;
+	case Opt_bulk_read:
+		c->mount_opts.bulk_read = 2;
+		c->bulk_read = 1;
+		break;
+	case Opt_no_bulk_read:
+		c->mount_opts.bulk_read = 1;
+		c->bulk_read = 0;
+		break;
+	case Opt_chk_data_crc:
+		c->mount_opts.chk_data_crc = 2;
+		c->no_chk_data_crc = 0;
+		break;
+	case Opt_no_chk_data_crc:
+		c->mount_opts.chk_data_crc = 1;
+		c->no_chk_data_crc = 1;
+		break;
+	case Opt_compr:
+		c->mount_opts.compr_type = result.uint_32;
+		c->mount_opts.override_compr = 1;
+		c->default_compr = c->mount_opts.compr_type;
+		break;
+	case Opt_assert:
+		c->assert_action = result.uint_32;
+		break;
+	case Opt_auth_key:
+		if (!is_remount) {
+			kfree(c->auth_key_name);
+			c->auth_key_name = param->string;
+			param->string = NULL;
 		}
+		break;
+	case Opt_auth_hash_name:
+		if (!is_remount) {
+			kfree(c->auth_hash_name);
+			c->auth_hash_name = param->string;
+			param->string = NULL;
 		}
+		break;
+	case Opt_ignore:
+		break;
 	}
 
 	return 0;
@@ -2003,21 +1937,30 @@ static void ubifs_put_super(struct super_block *sb)
 	mutex_unlock(&c->umount_mutex);
 }
 
-static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
+static int ubifs_reconfigure(struct fs_context *fc)
 {
+	struct super_block *sb = fc->root->d_sb;
 	int err;
 	struct ubifs_info *c = sb->s_fs_info;
+	struct ubifs_info *reconf = fc->s_fs_info;
 
 	sync_filesystem(sb);
-	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
+	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, fc->sb_flags);
 
-	err = ubifs_parse_options(c, data, 1);
-	if (err) {
-		ubifs_err(c, "invalid or unknown remount parameter");
-		return err;
-	}
+	/* Apply the mount option changes.
+	 *
+	 * [!] NOTE Replacing the auth_key_name and auth_hash_name is
+	 *     very dodgy without locking.  Previously it just leaked
+	 *     the old strings.  The strings only seem to be used
+	 *     during mounting, so don't reconfigure those.
+	 */
+	c->mount_opts		= reconf->mount_opts;
+	c->bulk_read		= reconf->bulk_read;
+	c->no_chk_data_crc	= reconf->no_chk_data_crc;
+	c->default_compr	= reconf->default_compr;
+	c->assert_action	= reconf->assert_action;
 
-	if (c->ro_mount && !(*flags & SB_RDONLY)) {
+	if (c->ro_mount && !(fc->sb_flags & SB_RDONLY)) {
 		if (c->ro_error) {
 			ubifs_msg(c, "cannot re-mount R/W due to prior errors");
 			return -EROFS;
@@ -2029,7 +1972,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
 		err = ubifs_remount_rw(c);
 		if (err)
 			return err;
-	} else if (!c->ro_mount && (*flags & SB_RDONLY)) {
+	} else if (!c->ro_mount && (fc->sb_flags & SB_RDONLY)) {
 		if (c->ro_error) {
 			ubifs_msg(c, "cannot re-mount R/O due to prior errors");
 			return -EROFS;
@@ -2062,14 +2005,13 @@ const struct super_operations ubifs_super_operations = {
 	.evict_inode   = ubifs_evict_inode,
 	.statfs        = ubifs_statfs,
 	.dirty_inode   = ubifs_dirty_inode,
-	.remount_fs    = ubifs_remount_fs,
 	.show_options  = ubifs_show_options,
 	.sync_fs       = ubifs_sync_fs,
 };
 
 /**
  * open_ubi - parse UBI device name string and open the UBI device.
- * @name: UBI volume name
+ * @fc: The filesystem parameters
  * @mode: UBI volume open mode
  *
  * The primary method of mounting UBIFS is by specifying the UBI volume
@@ -2086,15 +2028,13 @@ const struct super_operations ubifs_super_operations = {
  * returns UBI volume description object in case of success and a negative
  * error code in case of failure.
  */
-static struct ubi_volume_desc *open_ubi(const char *name, int mode)
+static struct ubi_volume_desc *open_ubi(struct fs_context *fc, int mode)
 {
 	struct ubi_volume_desc *ubi;
+	const char *name = fc->source;
 	int dev, vol;
 	char *endptr;
 
-	if (!name || !*name)
-		return ERR_PTR(-EINVAL);
-
 	/* First, try to open using the device node path method */
 	ubi = ubi_open_volume_path(name, mode);
 	if (!IS_ERR(ubi))
@@ -2102,14 +2042,14 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
 
 	/* Try the "nodev" method */
 	if (name[0] != 'u' || name[1] != 'b' || name[2] != 'i')
-		return ERR_PTR(-EINVAL);
+		goto invalid_source;
 
 	/* ubi:NAME method */
 	if ((name[3] == ':' || name[3] == '!') && name[4] != '\0')
 		return ubi_open_volume_nm(0, name + 4, mode);
 
 	if (!isdigit(name[3]))
-		return ERR_PTR(-EINVAL);
+		goto invalid_source;
 
 	dev = simple_strtoul(name + 3, &endptr, 0);
 
@@ -2121,7 +2061,7 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
 	if (*endptr == '_' && isdigit(endptr[1])) {
 		vol = simple_strtoul(endptr + 1, &endptr, 0);
 		if (*endptr != '\0')
-			return ERR_PTR(-EINVAL);
+			goto invalid_source;
 		return ubi_open_volume(dev, vol, mode);
 	}
 
@@ -2129,59 +2069,11 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
 	if ((*endptr == ':' || *endptr == '!') && endptr[1] != '\0')
 		return ubi_open_volume_nm(dev, ++endptr, mode);
 
-	return ERR_PTR(-EINVAL);
-}
-
-static struct ubifs_info *alloc_ubifs_info(struct ubi_volume_desc *ubi)
-{
-	struct ubifs_info *c;
-
-	c = kzalloc(sizeof(struct ubifs_info), GFP_KERNEL);
-	if (c) {
-		spin_lock_init(&c->cnt_lock);
-		spin_lock_init(&c->cs_lock);
-		spin_lock_init(&c->buds_lock);
-		spin_lock_init(&c->space_lock);
-		spin_lock_init(&c->orphan_lock);
-		init_rwsem(&c->commit_sem);
-		mutex_init(&c->lp_mutex);
-		mutex_init(&c->tnc_mutex);
-		mutex_init(&c->log_mutex);
-		mutex_init(&c->umount_mutex);
-		mutex_init(&c->bu_mutex);
-		mutex_init(&c->write_reserve_mutex);
-		init_waitqueue_head(&c->cmt_wq);
-		init_waitqueue_head(&c->reserve_space_wq);
-		atomic_set(&c->need_wait_space, 0);
-		c->buds = RB_ROOT;
-		c->old_idx = RB_ROOT;
-		c->size_tree = RB_ROOT;
-		c->orph_tree = RB_ROOT;
-		INIT_LIST_HEAD(&c->infos_list);
-		INIT_LIST_HEAD(&c->idx_gc);
-		INIT_LIST_HEAD(&c->replay_list);
-		INIT_LIST_HEAD(&c->replay_buds);
-		INIT_LIST_HEAD(&c->uncat_list);
-		INIT_LIST_HEAD(&c->empty_list);
-		INIT_LIST_HEAD(&c->freeable_list);
-		INIT_LIST_HEAD(&c->frdi_idx_list);
-		INIT_LIST_HEAD(&c->unclean_leb_list);
-		INIT_LIST_HEAD(&c->old_buds);
-		INIT_LIST_HEAD(&c->orph_list);
-		INIT_LIST_HEAD(&c->orph_new);
-		c->no_chk_data_crc = 1;
-		c->assert_action = ASSACT_RO;
-
-		c->highest_inum = UBIFS_FIRST_INO;
-		c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
-
-		ubi_get_volume_info(ubi, &c->vi);
-		ubi_get_device_info(c->vi.ubi_num, &c->di);
-	}
-	return c;
+invalid_source:
+	return ERR_PTR(invalf(fc, "Invalid source name"));
 }
 
-static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
+static int ubifs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct ubifs_info *c = sb->s_fs_info;
 	struct inode *root;
@@ -2195,10 +2087,6 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		goto out;
 	}
 
-	err = ubifs_parse_options(c, data, 0);
-	if (err)
-		goto out_close;
-
 	/*
 	 * UBIFS provides 'backing_dev_info' in order to disable read-ahead. For
 	 * UBIFS, I/O is not deferred, it is done immediately in read_folio,
@@ -2264,69 +2152,61 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 	return err;
 }
 
-static int sb_test(struct super_block *sb, void *data)
+static int sb_test(struct super_block *sb, struct fs_context *fc)
 {
-	struct ubifs_info *c1 = data;
+	struct ubifs_info *c1 = fc->s_fs_info;
 	struct ubifs_info *c = sb->s_fs_info;
 
 	return c->vi.cdev == c1->vi.cdev;
 }
 
-static int sb_set(struct super_block *sb, void *data)
-{
-	sb->s_fs_info = data;
-	return set_anon_super(sb, NULL);
-}
-
-static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
-			const char *name, void *data)
+static int ubifs_get_tree(struct fs_context *fc)
 {
 	struct ubi_volume_desc *ubi;
-	struct ubifs_info *c;
+	struct ubifs_info *c = fc->s_fs_info;
 	struct super_block *sb;
 	int err;
 
-	dbg_gen("name %s, flags %#x", name, flags);
+	if (!fc->source || !*fc->source)
+		return invalf(fc, "No source specified");
+
+	dbg_gen("name %s, flags %#x", fc->source, fc->sb_flags);
 
 	/*
 	 * Get UBI device number and volume ID. Mount it read-only so far
 	 * because this might be a new mount point, and UBI allows only one
 	 * read-write user at a time.
 	 */
-	ubi = open_ubi(name, UBI_READONLY);
+	ubi = open_ubi(fc, UBI_READONLY);
 	if (IS_ERR(ubi)) {
-		if (!(flags & SB_SILENT))
-			pr_err("UBIFS error (pid: %d): cannot open \"%s\", error %d",
-			       current->pid, name, (int)PTR_ERR(ubi));
-		return ERR_CAST(ubi);
+		err = PTR_ERR(ubi);
+		if (!(fc->sb_flags & SB_SILENT))
+			errorf(fc, "UBIFS error (pid: %d): cannot open \"%s\", error %d",
+			       current->pid, fc->source, err);
+		return err;
 	}
 
-	c = alloc_ubifs_info(ubi);
-	if (!c) {
-		err = -ENOMEM;
-		goto out_close;
-	}
+	ubi_get_volume_info(ubi, &c->vi);
+	ubi_get_device_info(c->vi.ubi_num, &c->di);
 
 	dbg_gen("opened ubi%d_%d", c->vi.ubi_num, c->vi.vol_id);
 
-	sb = sget(fs_type, sb_test, sb_set, flags, c);
+	sb = sget_fc(fc, sb_test, set_anon_super_fc);
 	if (IS_ERR(sb)) {
 		err = PTR_ERR(sb);
-		kfree(c);
 		goto out_close;
 	}
 
 	if (sb->s_root) {
-		struct ubifs_info *c1 = sb->s_fs_info;
-		kfree(c);
 		/* A new mount point for already mounted UBIFS */
+		c = sb->s_fs_info;
 		dbg_gen("this ubi volume is already mounted");
-		if (!!(flags & SB_RDONLY) != c1->ro_mount) {
+		if (!!(fc->sb_flags & SB_RDONLY) != c->ro_mount) {
 			err = -EBUSY;
 			goto out_deact;
 		}
 	} else {
-		err = ubifs_fill_super(sb, data, flags & SB_SILENT ? 1 : 0);
+		err = ubifs_fill_super(sb, fc);
 		if (err)
 			goto out_deact;
 		/* We do not support atime */
@@ -2340,13 +2220,81 @@ static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
 	/* 'fill_super()' opens ubi again so we must close it here */
 	ubi_close_volume(ubi);
 
-	return dget(sb->s_root);
+	fc->root = dget(sb->s_root);
+	return 0;
 
 out_deact:
 	deactivate_locked_super(sb);
 out_close:
 	ubi_close_volume(ubi);
-	return ERR_PTR(err);
+	return err;
+}
+
+static void ubifs_free_fc(struct fs_context *fc)
+{
+	struct ubifs_info *c = fc->s_fs_info;
+
+	if (c) {
+		kfree(c->auth_key_name);
+		kfree(c->auth_hash_name);
+		kfree(c);
+	}
+}
+
+static const struct fs_context_operations ubifs_context_ops = {
+	.free		= ubifs_free_fc,
+	.parse_param	= ubifs_parse_param,
+	.get_tree	= ubifs_get_tree,
+	.reconfigure	= ubifs_reconfigure,
+};
+
+static int ubifs_init_fs_context(struct fs_context *fc)
+{
+	struct ubifs_info *c;
+
+	c = kzalloc(sizeof(struct ubifs_info), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	spin_lock_init(&c->cnt_lock);
+	spin_lock_init(&c->cs_lock);
+	spin_lock_init(&c->buds_lock);
+	spin_lock_init(&c->space_lock);
+	spin_lock_init(&c->orphan_lock);
+	init_rwsem(&c->commit_sem);
+	mutex_init(&c->lp_mutex);
+	mutex_init(&c->tnc_mutex);
+	mutex_init(&c->log_mutex);
+	mutex_init(&c->umount_mutex);
+	mutex_init(&c->bu_mutex);
+	mutex_init(&c->write_reserve_mutex);
+	init_waitqueue_head(&c->cmt_wq);
+	init_waitqueue_head(&c->reserve_space_wq);
+	atomic_set(&c->need_wait_space, 0);
+	c->buds = RB_ROOT;
+	c->old_idx = RB_ROOT;
+	c->size_tree = RB_ROOT;
+	c->orph_tree = RB_ROOT;
+	INIT_LIST_HEAD(&c->infos_list);
+	INIT_LIST_HEAD(&c->idx_gc);
+	INIT_LIST_HEAD(&c->replay_list);
+	INIT_LIST_HEAD(&c->replay_buds);
+	INIT_LIST_HEAD(&c->uncat_list);
+	INIT_LIST_HEAD(&c->empty_list);
+	INIT_LIST_HEAD(&c->freeable_list);
+	INIT_LIST_HEAD(&c->frdi_idx_list);
+	INIT_LIST_HEAD(&c->unclean_leb_list);
+	INIT_LIST_HEAD(&c->old_buds);
+	INIT_LIST_HEAD(&c->orph_list);
+	INIT_LIST_HEAD(&c->orph_new);
+	c->no_chk_data_crc = 1;
+	c->assert_action = ASSACT_RO;
+	c->highest_inum = UBIFS_FIRST_INO;
+	c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
+
+	fc->s_fs_info = c;
+	fc->ops = &ubifs_context_ops;
+	return 0;
 }
 
 static void kill_ubifs_super(struct super_block *s)
@@ -2359,7 +2307,8 @@ static void kill_ubifs_super(struct super_block *s)
 static struct file_system_type ubifs_fs_type = {
 	.name    = "ubifs",
 	.owner   = THIS_MODULE,
-	.mount   = ubifs_mount,
+	.init_fs_context = ubifs_init_fs_context,
+	.parameters	= ubifs_fs_param_spec,
 	.kill_sb = kill_ubifs_super,
 };
 MODULE_ALIAS_FS("ubifs");
-- 
2.46.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Eric Sandeen]

On 9/26/24 3:36 PM, Eric Sandeen wrote:
> From: David Howells <dhowells@redhat.com>
> 
> Convert the ubifs filesystem to the new internal mount API as the old
> one will be obsoleted and removed.  This allows greater flexibility in
> communication of mount parameters between userspace, the VFS and the
> filesystem.
> 
> See Documentation/filesystems/mount_api.txt for more information.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> [sandeen: forward-port old patch]
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> cc: Richard Weinberger <richard@nod.at>
> cc: Zhihao Cheng <chengzhihao1@huawei.com>
> cc: linux-mtd@lists.infradead.org

Just a note, I simply picked up David's old patch from his git branch at

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/patch/?id=efee75ecdcb8f37afdedbe79a77ee571325b57d5

and forward-ported it. It's build tested only, so review and testing
would be most appreciated.

Aside from some minor changes to the internal mount API that
required adjustment, there was also a new compression type and a couple
additional ubifs_info members that needed initialization in
ubifs_init_fs_context.

Thanks!
-Eric


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Christian Brauner]

On Thu, 26 Sep 2024 15:36:04 -0500, Eric Sandeen wrote:
> Convert the ubifs filesystem to the new internal mount API as the old
> one will be obsoleted and removed.  This allows greater flexibility in
> communication of mount parameters between userspace, the VFS and the
> filesystem.
> 
> See Documentation/filesystems/mount_api.txt for more information.
> 
> [...]

Applied to the vfs.mount.api.v6.13 branch of the vfs/vfs.git tree.
Patches in the vfs.mount.api.v6.13 branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.mount.api.v6.13

[1/1] ubifs: Convert ubifs to use the new mount API
      https://git.kernel.org/vfs/vfs/c/10fe979418ac

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Zhihao Cheng]

在 2024/9/27 4:36, Eric Sandeen 写道:
Hi Eric, two comments below.
> From: David Howells <dhowells@redhat.com>
> 
> Convert the ubifs filesystem to the new internal mount API as the old
> one will be obsoleted and removed.  This allows greater flexibility in
> communication of mount parameters between userspace, the VFS and the
> filesystem.
> 
> See Documentation/filesystems/mount_api.txt for more information.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> [sandeen: forward-port old patch]
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> cc: Richard Weinberger <richard@nod.at>
> cc: Zhihao Cheng <chengzhihao1@huawei.com>
> cc: linux-mtd@lists.infradead.org
> ---
>   fs/ubifs/super.c | 459 +++++++++++++++++++++--------------------------
>   1 file changed, 204 insertions(+), 255 deletions(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 291583005dd1..cf2e9104baff 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
[...]
> @@ -963,7 +963,7 @@ static int check_volume_empty(struct ubifs_info *c)
>    * Opt_no_bulk_read: disable bulk-reads
>    * Opt_chk_data_crc: check CRCs when reading data nodes
>    * Opt_no_chk_data_crc: do not check CRCs when reading data nodes
> - * Opt_override_compr: override default compressor
> + * Opt_compr: override default compressor
>    * Opt_assert: set ubifs_assert() action
>    * Opt_auth_key: The key name used for authentication
>    * Opt_auth_hash_name: The hash type used for authentication
> @@ -976,54 +976,46 @@ enum {
>   	Opt_no_bulk_read,
>   	Opt_chk_data_crc,
>   	Opt_no_chk_data_crc,
> -	Opt_override_compr,
> +	Opt_compr,

IMO, I prefer the old name 'Opt_override_compr', it looks more closer to 
the semantics. UBIFS already set the default compressor in 
'c->default_compr', the 'Opt_override_compr' is used to overwrite it.

[...]

> -static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
> +static int ubifs_reconfigure(struct fs_context *fc)
>   {
> +	struct super_block *sb = fc->root->d_sb;
>   	int err;
>   	struct ubifs_info *c = sb->s_fs_info;
> +	struct ubifs_info *reconf = fc->s_fs_info;
>   
>   	sync_filesystem(sb);
> -	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
> +	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, fc->sb_flags);
>   
> -	err = ubifs_parse_options(c, data, 1);
> -	if (err) {
> -		ubifs_err(c, "invalid or unknown remount parameter");
> -		return err;
> -	}
> +	/* Apply the mount option changes.
> +	 *
> +	 * [!] NOTE Replacing the auth_key_name and auth_hash_name is
> +	 *     very dodgy without locking.  Previously it just leaked
> +	 *     the old strings.  The strings only seem to be used
> +	 *     during mounting, so don't reconfigure those.
> +	 */
> +	c->mount_opts		= reconf->mount_opts;
> +	c->bulk_read		= reconf->bulk_read;
> +	c->no_chk_data_crc	= reconf->no_chk_data_crc;
> +	c->default_compr	= reconf->default_compr;

We cannot overwrite old configurations with non-fully initialized new 
configurations directly, otherwise some old options will disappear, for 
example:
[root@localhost ~]# mount -ocompr=lzo /dev/ubi0_0 temp
[root@localhost ~]# mount | grep ubifs
/dev/ubi0_0 on /root/temp type ubifs 
(rw,relatime,compr=lzo,assert=read-only,ubi=0,vol=0)
The compressor is set as lzo.
[root@localhost ~]# mount -oremount /dev/ubi0_0 temp
[root@localhost ~]# mount | grep ubifs
/dev/ubi0_0 on /root/temp type ubifs 
(rw,relatime,assert=read-only,ubi=0,vol=0)

The compressor is not lzo anymore.

> +	c->assert_action	= reconf->assert_action;
>   
> -	if (c->ro_mount && !(*flags & SB_RDONLY)) {
> +	if (c->ro_mount && !(fc->sb_flags & SB_RDONLY)) {
>   		if (c->ro_error) {
>   			ubifs_msg(c, "cannot re-mount R/W due to prior errors");
>   			return -EROFS;



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Zhihao Cheng]

在 2024/9/27 22:12, Zhihao Cheng 写道:
> 在 2024/9/27 4:36, Eric Sandeen 写道:
> Hi Eric, two comments below.
>> From: David Howells <dhowells@redhat.com>
>>
>> Convert the ubifs filesystem to the new internal mount API as the old
>> one will be obsoleted and removed.  This allows greater flexibility in
>> communication of mount parameters between userspace, the VFS and the
>> filesystem.
>>
>> See Documentation/filesystems/mount_api.txt for more information.
>>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> [sandeen: forward-port old patch]
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> cc: Richard Weinberger <richard@nod.at>
>> cc: Zhihao Cheng <chengzhihao1@huawei.com>
>> cc: linux-mtd@lists.infradead.org
>> ---
>>   fs/ubifs/super.c | 459 +++++++++++++++++++++--------------------------
>>   1 file changed, 204 insertions(+), 255 deletions(-)
>>

BTW, I have tested this patch with xfstests-dev, the result looks good.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Eric Sandeen]

On 9/27/24 9:12 AM, Zhihao Cheng wrote:
> 在 2024/9/27 4:36, Eric Sandeen 写道:
> Hi Eric, two comments below.
>> From: David Howells <dhowells@redhat.com>
>>
>> Convert the ubifs filesystem to the new internal mount API as the old
>> one will be obsoleted and removed.  This allows greater flexibility in
>> communication of mount parameters between userspace, the VFS and the
>> filesystem.
>>
>> See Documentation/filesystems/mount_api.txt for more information.
>>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> [sandeen: forward-port old patch]
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> cc: Richard Weinberger <richard@nod.at>
>> cc: Zhihao Cheng <chengzhihao1@huawei.com>
>> cc: linux-mtd@lists.infradead.org
>> ---
>>   fs/ubifs/super.c | 459 +++++++++++++++++++++--------------------------
>>   1 file changed, 204 insertions(+), 255 deletions(-)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 291583005dd1..cf2e9104baff 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
> [...]
>> @@ -963,7 +963,7 @@ static int check_volume_empty(struct ubifs_info *c)
>>    * Opt_no_bulk_read: disable bulk-reads
>>    * Opt_chk_data_crc: check CRCs when reading data nodes
>>    * Opt_no_chk_data_crc: do not check CRCs when reading data nodes
>> - * Opt_override_compr: override default compressor
>> + * Opt_compr: override default compressor
>>    * Opt_assert: set ubifs_assert() action
>>    * Opt_auth_key: The key name used for authentication
>>    * Opt_auth_hash_name: The hash type used for authentication
>> @@ -976,54 +976,46 @@ enum {
>>       Opt_no_bulk_read,
>>       Opt_chk_data_crc,
>>       Opt_no_chk_data_crc,
>> -    Opt_override_compr,
>> +    Opt_compr,
> 
> IMO, I prefer the old name 'Opt_override_compr', it looks more closer to the semantics. UBIFS already set the default compressor in 'c->default_compr', the 'Opt_override_compr' is used to overwrite it.

Fair enough - TBH I did not notice that and not sure why David changed it, easy
enough to change it back!
 
> [...]
> 
>> -static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
>> +static int ubifs_reconfigure(struct fs_context *fc)
>>   {
>> +    struct super_block *sb = fc->root->d_sb;
>>       int err;
>>       struct ubifs_info *c = sb->s_fs_info;
>> +    struct ubifs_info *reconf = fc->s_fs_info;
>>         sync_filesystem(sb);
>> -    dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
>> +    dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, fc->sb_flags);
>>   -    err = ubifs_parse_options(c, data, 1);
>> -    if (err) {
>> -        ubifs_err(c, "invalid or unknown remount parameter");
>> -        return err;
>> -    }
>> +    /* Apply the mount option changes.
>> +     *
>> +     * [!] NOTE Replacing the auth_key_name and auth_hash_name is
>> +     *     very dodgy without locking.  Previously it just leaked
>> +     *     the old strings.  The strings only seem to be used
>> +     *     during mounting, so don't reconfigure those.
>> +     */
>> +    c->mount_opts        = reconf->mount_opts;
>> +    c->bulk_read        = reconf->bulk_read;
>> +    c->no_chk_data_crc    = reconf->no_chk_data_crc;
>> +    c->default_compr    = reconf->default_compr;
> 
> We cannot overwrite old configurations with non-fully initialized new configurations directly, otherwise some old options will disappear, for example:
> [root@localhost ~]# mount -ocompr=lzo /dev/ubi0_0 temp
> [root@localhost ~]# mount | grep ubifs
> /dev/ubi0_0 on /root/temp type ubifs (rw,relatime,compr=lzo,assert=read-only,ubi=0,vol=0)
> The compressor is set as lzo.
> [root@localhost ~]# mount -oremount /dev/ubi0_0 temp
> [root@localhost ~]# mount | grep ubifs
> /dev/ubi0_0 on /root/temp type ubifs (rw,relatime,assert=read-only,ubi=0,vol=0)
> The compressor is not lzo anymore.

Ah, yes. reconfigure is surprisingly tricky at times for reasons like this.

Is mount here from busybox by chance? I think usually util-linux mount respecifies every
existing mount option on remount which tends to hide this sort of thing; you could
double check this by stracing fsconfig calls during remount.

That said, we can preserve mount options internally as well. Does this patch on top
of the first one fix it for you?

(the other option would be to make an ->fs_private struct that holds only mount
options, rather than re-using s_fs_info as it does now - dhowells, any thoughts?)

Thanks for the review and testing!


diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index cf2e9104baff..be8154359772 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2256,44 +2256,56 @@ static int ubifs_init_fs_context(struct fs_context *fc)
 	if (!c)
 		return -ENOMEM;
 
-	spin_lock_init(&c->cnt_lock);
-	spin_lock_init(&c->cs_lock);
-	spin_lock_init(&c->buds_lock);
-	spin_lock_init(&c->space_lock);
-	spin_lock_init(&c->orphan_lock);
-	init_rwsem(&c->commit_sem);
-	mutex_init(&c->lp_mutex);
-	mutex_init(&c->tnc_mutex);
-	mutex_init(&c->log_mutex);
-	mutex_init(&c->umount_mutex);
-	mutex_init(&c->bu_mutex);
-	mutex_init(&c->write_reserve_mutex);
-	init_waitqueue_head(&c->cmt_wq);
-	init_waitqueue_head(&c->reserve_space_wq);
-	atomic_set(&c->need_wait_space, 0);
-	c->buds = RB_ROOT;
-	c->old_idx = RB_ROOT;
-	c->size_tree = RB_ROOT;
-	c->orph_tree = RB_ROOT;
-	INIT_LIST_HEAD(&c->infos_list);
-	INIT_LIST_HEAD(&c->idx_gc);
-	INIT_LIST_HEAD(&c->replay_list);
-	INIT_LIST_HEAD(&c->replay_buds);
-	INIT_LIST_HEAD(&c->uncat_list);
-	INIT_LIST_HEAD(&c->empty_list);
-	INIT_LIST_HEAD(&c->freeable_list);
-	INIT_LIST_HEAD(&c->frdi_idx_list);
-	INIT_LIST_HEAD(&c->unclean_leb_list);
-	INIT_LIST_HEAD(&c->old_buds);
-	INIT_LIST_HEAD(&c->orph_list);
-	INIT_LIST_HEAD(&c->orph_new);
-	c->no_chk_data_crc = 1;
-	c->assert_action = ASSACT_RO;
-	c->highest_inum = UBIFS_FIRST_INO;
-	c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
+	if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) {
+		spin_lock_init(&c->cnt_lock);
+		spin_lock_init(&c->cs_lock);
+		spin_lock_init(&c->buds_lock);
+		spin_lock_init(&c->space_lock);
+		spin_lock_init(&c->orphan_lock);
+		init_rwsem(&c->commit_sem);
+		mutex_init(&c->lp_mutex);
+		mutex_init(&c->tnc_mutex);
+		mutex_init(&c->log_mutex);
+		mutex_init(&c->umount_mutex);
+		mutex_init(&c->bu_mutex);
+		mutex_init(&c->write_reserve_mutex);
+		init_waitqueue_head(&c->cmt_wq);
+		init_waitqueue_head(&c->reserve_space_wq);
+		atomic_set(&c->need_wait_space, 0);
+		c->buds = RB_ROOT;
+		c->old_idx = RB_ROOT;
+		c->size_tree = RB_ROOT;
+		c->orph_tree = RB_ROOT;
+		INIT_LIST_HEAD(&c->infos_list);
+		INIT_LIST_HEAD(&c->idx_gc);
+		INIT_LIST_HEAD(&c->replay_list);
+		INIT_LIST_HEAD(&c->replay_buds);
+		INIT_LIST_HEAD(&c->uncat_list);
+		INIT_LIST_HEAD(&c->empty_list);
+		INIT_LIST_HEAD(&c->freeable_list);
+		INIT_LIST_HEAD(&c->frdi_idx_list);
+		INIT_LIST_HEAD(&c->unclean_leb_list);
+		INIT_LIST_HEAD(&c->old_buds);
+		INIT_LIST_HEAD(&c->orph_list);
+		INIT_LIST_HEAD(&c->orph_new);
+		c->no_chk_data_crc = 1;
+		c->assert_action = ASSACT_RO;
+		c->highest_inum = UBIFS_FIRST_INO;
+		c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
+	} else {
+		struct ubifs_info *c_orig = fc->root->d_sb->s_fs_info;
+
+		/* Preserve existing mount options across remounts */
+		c->mount_opts           = c_orig->mount_opts;
+		c->bulk_read            = c_orig->bulk_read;
+		c->no_chk_data_crc      = c_orig->no_chk_data_crc;
+		c->default_compr        = c_orig->default_compr;
+		c->assert_action        = c_orig->assert_action;
+	}
 
 	fc->s_fs_info = c;
 	fc->ops = &ubifs_context_ops;
+
 	return 0;
 }
 




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Zhihao Cheng]

在 2024/9/27 23:56, Eric Sandeen 写道:
> On 9/27/24 9:12 AM, Zhihao Cheng wrote:
>> 在 2024/9/27 4:36, Eric Sandeen 写道:
>> Hi Eric, two comments below.
>>> From: David Howells <dhowells@redhat.com>
>>>
>>> Convert the ubifs filesystem to the new internal mount API as the old
>>> one will be obsoleted and removed.  This allows greater flexibility in
>>> communication of mount parameters between userspace, the VFS and the
>>> filesystem.
>>>
>>> See Documentation/filesystems/mount_api.txt for more information.
>>>

[...]
>>
>> We cannot overwrite old configurations with non-fully initialized new configurations directly, otherwise some old options will disappear, for example:
>> [root@localhost ~]# mount -ocompr=lzo /dev/ubi0_0 temp
>> [root@localhost ~]# mount | grep ubifs
>> /dev/ubi0_0 on /root/temp type ubifs (rw,relatime,compr=lzo,assert=read-only,ubi=0,vol=0)
>> The compressor is set as lzo.
>> [root@localhost ~]# mount -oremount /dev/ubi0_0 temp
>> [root@localhost ~]# mount | grep ubifs
>> /dev/ubi0_0 on /root/temp type ubifs (rw,relatime,assert=read-only,ubi=0,vol=0)
>> The compressor is not lzo anymore.
> 
> Ah, yes. reconfigure is surprisingly tricky at times for reasons like this.
> 
> Is mount here from busybox by chance? I think usually util-linux mount respecifies every
> existing mount option on remount which tends to hide this sort of thing; you could
> double check this by stracing fsconfig calls during remount.

I think we shouldn't depend on the userspace utils, let's just consider 
the semantics of remounting from the view of syscall, because linux user 
can call remount by syscall directly.
> 
> That said, we can preserve mount options internally as well. Does this patch on top
> of the first one fix it for you?
> 
> (the other option would be to make an ->fs_private struct that holds only mount
> options, rather than re-using s_fs_info as it does now - dhowells, any thoughts?)
> 
> Thanks for the review and testing!
> 
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index cf2e9104baff..be8154359772 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -2256,44 +2256,56 @@ static int ubifs_init_fs_context(struct fs_context *fc)
>   	if (!c)
>   		return -ENOMEM;
>   
[...]
> +		INIT_LIST_HEAD(&c->orph_new);
> +		c->no_chk_data_crc = 1;
> +		c->assert_action = ASSACT_RO;
> +		c->highest_inum = UBIFS_FIRST_INO;
> +		c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
> +	} else {
> +		struct ubifs_info *c_orig = fc->root->d_sb->s_fs_info;
> +
> +		/* Preserve existing mount options across remounts */
> +		c->mount_opts           = c_orig->mount_opts;
> +		c->bulk_read            = c_orig->bulk_read;
> +		c->no_chk_data_crc      = c_orig->no_chk_data_crc;
> +		c->default_compr        = c_orig->default_compr;
> +		c->assert_action        = c_orig->assert_action;
> +	}
>   
>   	fc->s_fs_info = c;
>   	fc->ops = &ubifs_context_ops;
> +
>   	return 0;
>   }
>   
> 

Above modification can fix the problem, but it looks not so clean.
There are two main steps for mount/remount in the new API framework:
1. Get filesystem configurations by parsing the paramaters from the user 
data.
2. Apply the filesystem configurations to superblock(For mount, a 
superblock should be allocated first.).

So, how about doing that like ext4 does:
1) the filesystem specified superblock is allocated in fill_super(), not 
in init_fs_context(), it looks more reasonable, because filesystem 
doesn't need a new private superblock in remounting process. But, UBIFS 
has onething different, sb_test() needs volume information which comes 
from private superblock, so UBFIS private superblock(struct ubifs_info) 
is allocated in ubifs_mount(). Here, I prefer to keep private superblock 
allocation still in ubifs_mount(for new mount API, it is called 
ubifs_get_tree).
2) the filesystem configurations(contains mount options) are stored in a 
separated structure(in ext4, it is called ext4_fs_context), which is 
allocated in init_fs_context(). For UBIFS, we can define a similar 
structure to store the filesystem configurations.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Zhihao Cheng]

在 2024/9/29 9:57, Zhihao Cheng 写道:
> 在 2024/9/27 23:56, Eric Sandeen 写道:
>> On 9/27/24 9:12 AM, Zhihao Cheng wrote:
>>> 在 2024/9/27 4:36, Eric Sandeen 写道:
>>> Hi Eric, two comments below.
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> Convert the ubifs filesystem to the new internal mount API as the old
>>>> one will be obsoleted and removed.  This allows greater flexibility in
>>>> communication of mount parameters between userspace, the VFS and the
>>>> filesystem.
>>>>
>>>> See Documentation/filesystems/mount_api.txt for more information.
>>>>
> 
> [...]
>>>
>>> We cannot overwrite old configurations with non-fully initialized new 
>>> configurations directly, otherwise some old options will disappear, 
>>> for example:
>>> [root@localhost ~]# mount -ocompr=lzo /dev/ubi0_0 temp
>>> [root@localhost ~]# mount | grep ubifs
>>> /dev/ubi0_0 on /root/temp type ubifs 
>>> (rw,relatime,compr=lzo,assert=read-only,ubi=0,vol=0)
>>> The compressor is set as lzo.
>>> [root@localhost ~]# mount -oremount /dev/ubi0_0 temp
>>> [root@localhost ~]# mount | grep ubifs
>>> /dev/ubi0_0 on /root/temp type ubifs 
>>> (rw,relatime,assert=read-only,ubi=0,vol=0)
>>> The compressor is not lzo anymore.
>>
>> Ah, yes. reconfigure is surprisingly tricky at times for reasons like 
>> this.
>>
>> Is mount here from busybox by chance? I think usually util-linux mount 
>> respecifies every
>> existing mount option on remount which tends to hide this sort of 
>> thing; you could
>> double check this by stracing fsconfig calls during remount.
> 
> I think we shouldn't depend on the userspace utils, let's just consider 
> the semantics of remounting from the view of syscall, because linux user 
> can call remount by syscall directly.
>>
>> That said, we can preserve mount options internally as well. Does this 
>> patch on top
>> of the first one fix it for you?
>>
>> (the other option would be to make an ->fs_private struct that holds 
>> only mount
>> options, rather than re-using s_fs_info as it does now - dhowells, any 
>> thoughts?)
>>
>> Thanks for the review and testing!
>>
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index cf2e9104baff..be8154359772 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -2256,44 +2256,56 @@ static int ubifs_init_fs_context(struct 
>> fs_context *fc)
>>       if (!c)
>>           return -ENOMEM;
> [...]
>> +        INIT_LIST_HEAD(&c->orph_new);
>> +        c->no_chk_data_crc = 1;
>> +        c->assert_action = ASSACT_RO;
>> +        c->highest_inum = UBIFS_FIRST_INO;
>> +        c->lhead_lnum = c->ltail_lnum = UBIFS_LOG_LNUM;
>> +    } else {
>> +        struct ubifs_info *c_orig = fc->root->d_sb->s_fs_info;
>> +
>> +        /* Preserve existing mount options across remounts */
>> +        c->mount_opts           = c_orig->mount_opts;
>> +        c->bulk_read            = c_orig->bulk_read;
>> +        c->no_chk_data_crc      = c_orig->no_chk_data_crc;
>> +        c->default_compr        = c_orig->default_compr;
>> +        c->assert_action        = c_orig->assert_action;
>> +    }
>>       fc->s_fs_info = c;
>>       fc->ops = &ubifs_context_ops;
>> +
>>       return 0;
>>   }
>>
> 
> Above modification can fix the problem, but it looks not so clean.
> There are two main steps for mount/remount in the new API framework:
> 1. Get filesystem configurations by parsing the paramaters from the user 
> data.
> 2. Apply the filesystem configurations to superblock(For mount, a 
> superblock should be allocated first.).
> 
> So, how about doing that like ext4 does:
> 1) the filesystem specified superblock is allocated in fill_super(), not 
> in init_fs_context(), it looks more reasonable, because filesystem 
> doesn't need a new private superblock in remounting process. But, UBIFS 
> has onething different, sb_test() needs volume information which comes 
> from private superblock, so UBFIS private superblock(struct ubifs_info) 
> is allocated in ubifs_mount(). Here, I prefer to keep private superblock 
> allocation still in ubifs_mount(for new mount API, it is called 
> ubifs_get_tree).

Notice, if you do that, please be careful to check the resource 
releasing problems in kinds of error handling branches. I have checked 
that in v1, it looks sophisticated.
> 2) the filesystem configurations(contains mount options) are stored in a 
> separated structure(in ext4, it is called ext4_fs_context), which is 
> allocated in init_fs_context(). For UBIFS, we can define a similar 
> structure to store the filesystem configurations.
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Eric Sandeen]

On 9/28/24 8:57 PM, Zhihao Cheng wrote:
> Above modification can fix the problem, but it looks not so clean.
> There are two main steps for mount/remount in the new API framework:
> 1. Get filesystem configurations by parsing the paramaters from the user data.
> 2. Apply the filesystem configurations to superblock(For mount, a superblock should be allocated first.).
> 
> So, how about doing that like ext4 does:
> 1) the filesystem specified superblock is allocated in fill_super(), not in init_fs_context(), it looks more reasonable, because filesystem doesn't need a new private superblock in remounting process. But, UBIFS has onething different, sb_test() needs volume information which comes from private superblock, so UBFIS private superblock(struct ubifs_info) is allocated in ubifs_mount(). Here, I prefer to keep private superblock allocation still in ubifs_mount(for new mount API, it is called ubifs_get_tree).
> 2) the filesystem configurations(contains mount options) are stored in a separated structure(in ext4, it is called ext4_fs_context), which is allocated in init_fs_context(). For UBIFS, we can define a similar structure to store the filesystem configurations.

Yes, this is what I had mentioned above, i.e. having a separate mount
context in ->fs_private raather than using ->fs_info, I can do it this
way if you prefer. I had just started with dhowell's original
implementation for speed. :)

Thanks,
-Eric


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Zhihao Cheng]

在 2024/9/30 0:46, Eric Sandeen 写道:
> On 9/28/24 8:57 PM, Zhihao Cheng wrote:
>> Above modification can fix the problem, but it looks not so clean.
>> There are two main steps for mount/remount in the new API framework:
>> 1. Get filesystem configurations by parsing the paramaters from the user data.
>> 2. Apply the filesystem configurations to superblock(For mount, a superblock should be allocated first.).
>>
>> So, how about doing that like ext4 does:
>> 1) the filesystem specified superblock is allocated in fill_super(), not in init_fs_context(), it looks more reasonable, because filesystem doesn't need a new private superblock in remounting process. But, UBIFS has onething different, sb_test() needs volume information which comes from private superblock, so UBFIS private superblock(struct ubifs_info) is allocated in ubifs_mount(). Here, I prefer to keep private superblock allocation still in ubifs_mount(for new mount API, it is called ubifs_get_tree).
>> 2) the filesystem configurations(contains mount options) are stored in a separated structure(in ext4, it is called ext4_fs_context), which is allocated in init_fs_context(). For UBIFS, we can define a similar structure to store the filesystem configurations.
> 
> Yes, this is what I had mentioned above, i.e. having a separate mount
> context in ->fs_private raather than using ->fs_info, I can do it this
> way if you prefer. I had just started with dhowell's original
> implementation for speed. :)

Fine, we've come to an agreement.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] ubifs: Convert ubifs to use the new mount API

[Eric Sandeen]

On 9/29/24 8:22 PM, Zhihao Cheng wrote:
> 在 2024/9/30 0:46, Eric Sandeen 写道:
>> On 9/28/24 8:57 PM, Zhihao Cheng wrote:
>>> Above modification can fix the problem, but it looks not so clean.
>>> There are two main steps for mount/remount in the new API framework:
>>> 1. Get filesystem configurations by parsing the paramaters from the user data.
>>> 2. Apply the filesystem configurations to superblock(For mount, a superblock should be allocated first.).
>>>
>>> So, how about doing that like ext4 does:
>>> 1) the filesystem specified superblock is allocated in fill_super(), not in init_fs_context(), it looks more reasonable, because filesystem doesn't need a new private superblock in remounting process. But, UBIFS has onething different, sb_test() needs volume information which comes from private superblock, so UBFIS private superblock(struct ubifs_info) is allocated in ubifs_mount(). Here, I prefer to keep private superblock allocation still in ubifs_mount(for new mount API, it is called ubifs_get_tree).
>>> 2) the filesystem configurations(contains mount options) are stored in a separated structure(in ext4, it is called ext4_fs_context), which is allocated in init_fs_context(). For UBIFS, we can define a similar structure to store the filesystem configurations.
>>
>> Yes, this is what I had mentioned above, i.e. having a separate mount
>> context in ->fs_private raather than using ->fs_info, I can do it this
>> way if you prefer. I had just started with dhowell's original
>> implementation for speed. :)
> 
> Fine, we've come to an agreement.
> 

Ok - I'm working on this, and I'll set up a virtual mtd device so I
can do some proper initial testing, given the changes from the original.

Thanks,
-Eric


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/