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'.

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

Reply via email to