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@xxxxxxxxxxxxxxx # 6.11+
> >>> Signed-off-by: Daniel Rosenberg <drosen@xxxxxxxxxx>
> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> >>> ---
> >>> 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
> >>
> >
