This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org --- fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type, + Opt_lazytime, + Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota, @@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"}, + {Opt_lazytime, "lazytime"}, + {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"}, @@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif + case Opt_lazytime: + sb->s_flags |= SB_LAZYTIME; + break; + case Opt_nolazytime: + sb->s_flags &= ~SB_LAZYTIME; + break; #ifdef CONFIG_QUOTA case Opt_quota: case Opt_usrquota:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Thanks,
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;#ifdef CONFIG_QUOTA case Opt_quota: case Opt_usrquota: -- 2.47.0.277.g8800431eea-goog
On 2024/11/13 5:39, Jaegeuk Kim via Linux-f2fs-devel wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Hi all,
Actually, if we want to enable lazytime option, we can use mount syscall as:
mount("/dev/vdb", "/mnt/test", "f2fs", MS_LAZYTIME, NULL);
or use shell script as:
mount -t f2fs -o lazytime /dev/vdb /mnt/test
IIUC, the reason why mount command can handle lazytime is, after 8c7f073aaeaa ("libmount: add support for MS_LAZYTIME"), mount command supports to map "lazytime" to MS_LAZYTIME, and use MS_LAZYTIME in parameter @mountflags of mount(2).
So, it looks we have alternative way to enable/disable lazytime feature after removing Opt_{no,}lazytime parsing in f2fs, do we really need this revert patch?
Thanks,
Thanks,
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;-- 2.47.0.277.g8800431eea-goog
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 11/19, Chao Yu wrote:
On 2024/11/13 5:39, Jaegeuk Kim via Linux-f2fs-devel wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Hi all,
Actually, if we want to enable lazytime option, we can use mount syscall as:
mount("/dev/vdb", "/mnt/test", "f2fs", MS_LAZYTIME, NULL);
or use shell script as:
mount -t f2fs -o lazytime /dev/vdb /mnt/test
IIUC, the reason why mount command can handle lazytime is, after 8c7f073aaeaa ("libmount: add support for MS_LAZYTIME"), mount command supports to map "lazytime" to MS_LAZYTIME, and use MS_LAZYTIME in parameter @mountflags of mount(2).
So, it looks we have alternative way to enable/disable lazytime feature after removing Opt_{no,}lazytime parsing in f2fs, do we really need this revert patch?
This is a regression of the below command. I don't think offering others are feasible.
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
Thanks,
Thanks,
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;-- 2.47.0.277.g8800431eea-goog
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 2024/11/20 4:48, Jaegeuk Kim wrote:
On 11/19, Chao Yu wrote:
On 2024/11/13 5:39, Jaegeuk Kim via Linux-f2fs-devel wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Hi all,
Actually, if we want to enable lazytime option, we can use mount syscall as:
mount("/dev/vdb", "/mnt/test", "f2fs", MS_LAZYTIME, NULL);
or use shell script as:
mount -t f2fs -o lazytime /dev/vdb /mnt/test
IIUC, the reason why mount command can handle lazytime is, after 8c7f073aaeaa ("libmount: add support for MS_LAZYTIME"), mount command supports to map "lazytime" to MS_LAZYTIME, and use MS_LAZYTIME in parameter @mountflags of mount(2).
So, it looks we have alternative way to enable/disable lazytime feature after removing Opt_{no,}lazytime parsing in f2fs, do we really need this revert patch?
This is a regression of the below command. I don't think offering others are feasible.
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
Alright, there are other options were removed along w/ removal of related feature. e.g.
1. io_bits=%u by commit 87161a2b0aed ("f2fs: deprecate io_bits") 2. whint_mode=%s by commit 930e2607638d ("f2fs: remove obsolete whint_mode")
Do we need to add these options handling back, and print "xxx options were deprecated" as we did in ("f2fs: kill heap-based allocation"), in order to avoid mount(......, "io_bits=%u" or "whint_mode=%s") command regression?
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 867b147eb957..329f317e6f09 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) clear_opt(sbi, DISCARD); break; case Opt_noheap: - set_opt(sbi, NOHEAP); - break; case Opt_heap: - clear_opt(sbi, NOHEAP); + f2fs_warn(sbi, "heap/no_heap options were deprecated"); break;
Thanks,
Thanks,
Thanks,
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;-- 2.47.0.277.g8800431eea-goog
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 11/20, Chao Yu wrote:
On 2024/11/20 4:48, Jaegeuk Kim wrote:
On 11/19, Chao Yu wrote:
On 2024/11/13 5:39, Jaegeuk Kim via Linux-f2fs-devel wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Hi all,
Actually, if we want to enable lazytime option, we can use mount syscall as:
mount("/dev/vdb", "/mnt/test", "f2fs", MS_LAZYTIME, NULL);
or use shell script as:
mount -t f2fs -o lazytime /dev/vdb /mnt/test
IIUC, the reason why mount command can handle lazytime is, after 8c7f073aaeaa ("libmount: add support for MS_LAZYTIME"), mount command supports to map "lazytime" to MS_LAZYTIME, and use MS_LAZYTIME in parameter @mountflags of mount(2).
So, it looks we have alternative way to enable/disable lazytime feature after removing Opt_{no,}lazytime parsing in f2fs, do we really need this revert patch?
This is a regression of the below command. I don't think offering others are feasible.
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
Alright, there are other options were removed along w/ removal of related feature. e.g.
- io_bits=%u by commit 87161a2b0aed ("f2fs: deprecate io_bits")
- whint_mode=%s by commit 930e2607638d ("f2fs: remove obsolete whint_mode")
Do we need to add these options handling back, and print "xxx options were deprecated" as we did in ("f2fs: kill heap-based allocation"), in order to avoid mount(......, "io_bits=%u" or "whint_mode=%s") command regression?
These are different from the generic lazytime option, and I haven't got any complaint on this removal.
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 867b147eb957..329f317e6f09 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -733,10 +733,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) clear_opt(sbi, DISCARD); break; case Opt_noheap:
set_opt(sbi, NOHEAP);break;clear_opt(sbi, NOHEAP);
f2fs_warn(sbi, "heap/no_heap options were deprecated"); break;Thanks,
Thanks,
Thanks,
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;-- 2.47.0.277.g8800431eea-goog
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 11/12/24 3:39 PM, Jaegeuk Kim wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Thanks,
I'm sorry, I missed this email. I will look into it more today.
As for f2fs new mount API support, I have been struggling with it for a long time, f2fs has been uniquely complex. The assumption that the superblock and on-disk features are known at option parsing time makes it much more difficult than most other filesystems.
But if there's a problem/regression with this commit, I have no objection to reverting the commit for now, and I'm sorry for the error.
-Eric
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;#ifdef CONFIG_QUOTA case Opt_quota: case Opt_usrquota: -- 2.47.0.277.g8800431eea-goog
On 11/20/24 8:27 AM, Eric Sandeen wrote:
On 11/12/24 3:39 PM, Jaegeuk Kim wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Thanks,
I'm sorry, I missed this email. I will look into it more today.
Ok, I see that I had not considered a direct mount call passing the lazytime option strings. :(
Using mount(8), "lazytime" is never passed as an option all the way to f2fs, nor is "nolazytime" -
# mount -o loop,nolazytime f2fsfile.img mnt # mount | grep lazytime /root/f2fs-test/f2fsfile.img on /root/f2fs-test/mnt type f2fs (rw,relatime,lazytime,seclabel,background_gc=on,nogc_merge,discard,discard_unit=block,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,barrier,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,checkpoint_merge,fsync_mode=posix,memory=normal,errors=continue)
(note that lazytime is still set despite -o nolazytime)
when mount(8) is using the new mount API, it does do fsconfig for (no)lazytime:
fsconfig(3, FSCONFIG_SET_FLAG, "nolazytime", NULL, 0) = 0
but that is consumed by the VFS and never sent into f2fs for parsing.
And because default_options() does:
sbi->sb->s_flags |= SB_LAZYTIME;
by default, it overrides the "nolazytime" that the vfs had previously handled.
I'm fairly sure that when mount(8) was using the old mount API (long ago) it also did not send in the lazytime option string - it sent it as a flag instead.
However - a direct call to mount(2) /will/ pass those options all the way to f2fs, and parse_options() does need to handle them there or it will be rejected as an invalid option.
(Note that f2fs is the only filesystem that attempts to handle lazytime within the filesystem itself):
[linux]# grep -r "lazytime" fs/*/ fs/f2fs/super.c: {Opt_lazytime, "lazytime"}, [linux]#
I'm not entirely sure how to untangle all this, but regressions are not acceptable, so please revert my commit for now.
Thanks, -Eric
As for f2fs new mount API support, I have been struggling with it for a long time, f2fs has been uniquely complex. The assumption that the superblock and on-disk features are known at option parsing time makes it much more difficult than most other filesystems.
But if there's a problem/regression with this commit, I have no objection to reverting the commit for now, and I'm sorry for the error.
-Eric
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;#ifdef CONFIG_QUOTA case Opt_quota: case Opt_usrquota: -- 2.47.0.277.g8800431eea-goog
On 11/20, Eric Sandeen wrote:
On 11/20/24 8:27 AM, Eric Sandeen wrote:
On 11/12/24 3:39 PM, Jaegeuk Kim wrote:
Hi Eric,
Could you please check this revert as it breaks the mount()? It seems F2FS needs to implement new mount support.
Thanks,
I'm sorry, I missed this email. I will look into it more today.
Ok, I see that I had not considered a direct mount call passing the lazytime option strings. :(
Using mount(8), "lazytime" is never passed as an option all the way to f2fs, nor is "nolazytime" -
# mount -o loop,nolazytime f2fsfile.img mnt # mount | grep lazytime /root/f2fs-test/f2fsfile.img on /root/f2fs-test/mnt type f2fs (rw,relatime,lazytime,seclabel,background_gc=on,nogc_merge,discard,discard_unit=block,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,barrier,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,checkpoint_merge,fsync_mode=posix,memory=normal,errors=continue)
(note that lazytime is still set despite -o nolazytime)
when mount(8) is using the new mount API, it does do fsconfig for (no)lazytime:
fsconfig(3, FSCONFIG_SET_FLAG, "nolazytime", NULL, 0) = 0
but that is consumed by the VFS and never sent into f2fs for parsing.
And because default_options() does:
sbi->sb->s_flags |= SB_LAZYTIME;
by default, it overrides the "nolazytime" that the vfs had previously handled.
I'm fairly sure that when mount(8) was using the old mount API (long ago) it also did not send in the lazytime option string - it sent it as a flag instead.
However - a direct call to mount(2) /will/ pass those options all the way to f2fs, and parse_options() does need to handle them there or it will be rejected as an invalid option.
(Note that f2fs is the only filesystem that attempts to handle lazytime within the filesystem itself):
[linux]# grep -r "lazytime" fs/*/ fs/f2fs/super.c: {Opt_lazytime, "lazytime"}, [linux]#
I'm not entirely sure how to untangle all this, but regressions are not acceptable, so please revert my commit for now.
Thanks for the explanation. At a glance, I thought it's caused that f2fs doesn't implement fs_context_operations. We'll take a look at how to support it.
Thanks, -Eric
As for f2fs new mount API support, I have been struggling with it for a long time, f2fs has been uniquely complex. The assumption that the superblock and on-disk features are known at option parsing time makes it much more difficult than most other filesystems.
But if there's a problem/regression with this commit, I have no objection to reverting the commit for now, and I'm sorry for the error.
-Eric
On 11/12, Jaegeuk Kim wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 49519439b770..35c4394e4fc6 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -150,6 +150,8 @@ enum { Opt_mode, Opt_fault_injection, Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime, Opt_quota, Opt_noquota, Opt_usrquota,
@@ -226,6 +228,8 @@ static match_table_t f2fs_tokens = { {Opt_mode, "mode=%s"}, {Opt_fault_injection, "fault_injection=%u"}, {Opt_fault_type, "fault_type=%u"},
- {Opt_lazytime, "lazytime"},
- {Opt_nolazytime, "nolazytime"}, {Opt_quota, "quota"}, {Opt_noquota, "noquota"}, {Opt_usrquota, "usrquota"},
@@ -922,6 +926,12 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) f2fs_info(sbi, "fault_type options not supported"); break; #endif
case Opt_lazytime:sb->s_flags |= SB_LAZYTIME;break;case Opt_nolazytime:sb->s_flags &= ~SB_LAZYTIME;break;#ifdef CONFIG_QUOTA case Opt_quota: case Opt_usrquota: -- 2.47.0.277.g8800431eea-goog
Hello:
This patch was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim jaegeuk@kernel.org:
On Tue, 12 Nov 2024 01:08:20 +0000 you wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
[...]
Here is the summary with links: - [f2fs-dev] Revert "f2fs: remove unreachable lazytime mount option parsing" https://git.kernel.org/jaegeuk/f2fs/c/f8bed73c6c52
You are awesome, thank you!
On 2024/11/12 9:08, Jaegeuk Kim via Linux-f2fs-devel wrote:
This reverts commit 54f43a10fa257ad4af02a1d157fefef6ebcfa7dc.
The above commit broke the lazytime mount, given
mount("/dev/vdb", "/mnt/test", "f2fs", 0, "lazytime");
CC: stable@vger.kernel.org # 6.11+ Signed-off-by: Daniel Rosenberg drosen@google.com Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
Reviewed-by: Chao Yu chao@kernel.org
Thanks,
linux-stable-mirror@lists.linaro.org
-
Chao Yu -
Eric Sandeen -
Jaegeuk Kim -
patchwork-bot+f2fs＠kernel.org