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

Zhihao Cheng
2024-09-27T14:13:24Z

在 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;