[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 at 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 at localhost ~]# mount -ocompr=lzo /dev/ubi0_0 temp
>> [root at 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 at localhost ~]# mount -oremount /dev/ubi0_0 temp
>> [root at 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.