On 2018/9/20 7:54, cgxu519 wrote:
> On 9/19/18 10:02 PM, Chao Yu wrote:
>> On 2018/9/18 21:47, cgxu519 wrote:
>>> On 09/18/2018 09:20 PM, Chao Yu wrote:
>>>> On 2018/9/18 14:23, Chengguang Xu wrote:
>>>>> Currently we set default value to options before parsing remount options,
>>>>> it will cause unexpected change of options which were specified in first
>>>>> mount.
>>>> Can you check below commit? It looks w/o it we may lose default option
>>>> after
>>>> remount.
>>>>
>>>> 498c5e9fcd10 ("f2fs: add default mount options to remount")
>>> Hi Chao,
>> Hi Chengguang,
>>
>>> It looks like there was a bug in remount at that time, but I think the fix
>>> above
>>> is not correct.
>>>
>>> from the patch '498c5e9fcd10', I think it was caused by clearing
>>> sbi->mount_opt.opt before
>>>
>>> parsing. I think remount should not change the options which were specified
>>> by
>>> user in
>>>
>>> previous mount unless user specifies in remount.
>> Can you check description in manual of mount.
>>
>> IIRC, old mount option will be record into /etc/mtab or /proc/mounts (for
>> adapting namespace feature), with command of "mount -o remount,rw /dir", old
>> mount options can be loaded from above config file, and merge them with new
>> specified options.
>>
>> Even we kill old mount options by call default_options() in ->remount, user's
>> old mount option can still be set through parameters.
>
> Please let me show you different behaviors before/after this patch.
>
>
> [root@x201 cgxu]# uname -a
> Linux x201 4.19.0-rc2+ #51 SMP Wed Sep 19 22:41:20 CST 2018 x86_64
> x86_64 x86_64 GNU/Linux
>
>
> Before:
>
> [root@x201 cgxu]# mount -o fsync_mode=strict /dev/mapper/test-test1
> /mnt/test/test1
> [root@x201 cgxu]# mount | grep test1
> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
> (rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
>
> [root@x201 cgxu]# mount -o remount,background_gc=sync
> /dev/mapper/test-test1 /mnt/test/test1
> [root@x201 cgxu]# mount | grep test1
> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
> (rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=default,fsync_mode=posix)
>
>
> I only changed background_gc in ->remount, but fsync_mode is implicitly
> set to default value 'posix'.
IMO, the resul is as expected, let's referenced description in manual of mount:
The remount functionality follows the standard way how the
mount command works with options from fstab. It means the mount command
doesn't read fstab (or mtab) only when a device and
dir are fully specified.
mount -o remount,rw /dev/foo /dir
After this call all old mount options are replaced and
arbitrary stuff from fstab is ignored, except the loop= option which is
internally generated and maintained by the mount command.
mount -o remount,rw /dir
After this call mount reads fstab (or mtab) and merges these
options with options from command line ( -o ).
So if you want to keep old mount option, you should use:
mount -o remount,background_gc=sync /mnt/test/test1
instead of
mount -o remount,background_gc=sync /dev/mapper/test-test1 /mnt/test/test1
Thanks,
>
> After:
>
> [root@x201 linus]# mount -o fsync_mode=strict /dev/mapper/test-test1
> /mnt/test/test1
> [root@x201 linus]# mount | grep test1
> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
> (rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
>
> [root@x201 linus]# mount -o remount,background_gc=sync
> /dev/mapper/test-test1 /mnt/test/test1
> [root@x201 linus]# mount | grep test1
> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
> (rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
>
> This time fysnc_mode looks fine.
>
>>
>> But the problem here is, some old parameter can be configured via sysfs, if
>> we
>> reset them in default_options(), we will lose them forever after remount.
>>
>> If you have no other opinion about this, could you adapt your commit log?
>>
>
> .
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
