[f2fs-dev] [PATCH] f2fs: remove unreachable lazytime mount option parsing
The lazytime/nolazytime options are now handled in the VFS, and are
never seen in filesystem parsers, so remove handling of these
options from f2fs.
Note: when lazytime support was added in 6d94c74ab85f it made
lazytime the default in default_options() - as a result, lazytime
cannot be disabled (because Opt_nolazytime is never seen in f2fs
parsing).
If lazytime is desired to be configurable, and default off is OK,
default_options() could be updated to stop setting it by default
and allow mount option control.
Signed-off-by: Eric Sandeen
---
(I came across this when looking at mount API conversion, which still
has not gotten very far, sorry!)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f1b3647a998..12bf7f014fc4 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -151,8 +151,6 @@ enum {
Opt_mode,
Opt_fault_injection,
Opt_fault_type,
- Opt_lazytime,
- Opt_nolazytime,
Opt_quota,
Opt_noquota,
Opt_usrquota,
@@ -229,8 +227,6 @@ 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"},
@@ -918,12 +914,6 @@ 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:
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 0/9] f2fs: new mount API conversion
Just FWIW - I had missed this thread when I got temporarily unsubscribed from fsdevel. I have a series that I was hacking on for this same work, at https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/commit/?h=f2fs-mount-api but it's very rough and almost certainly contains bugs. It may or may not be of any help to you, but just FYI. I'll try to help review/test your series since I tried to solve this as well, but I never completed the work. :) Thanks, -Eric On 8/27/24 6:47 AM, Hongbo Li wrote: > Does there exist CI test for f2fs? I can only write the mount test for f2fs > refer to tests/ext4/053. And I have tested this in local. > > Thanks, > Hongbo > > On 2024/8/14 10:39, Hongbo Li wrote: >> Since many filesystems have done the new mount API conversion, >> we introduce the new mount API conversion in f2fs. >> >> The series can be applied on top of the current mainline tree >> and the work is based on the patches from Lukas Czerner (has >> done this in ext4[1]). His patch give me a lot of ideas. >> >> Here is a high level description of the patchset: >> >> 1. Prepare the f2fs mount parameters required by the new mount >> API and use it for parsing, while still using the old API to >> get mount options string. Split the parameter parsing and >> validation of the parse_options helper into two separate >> helpers. >> >> f2fs: Add fs parameter specifications for mount options >> f2fs: move the option parser into handle_mount_opt >> f2fs: move option validation into a separate helper >> >> 2. Remove the use of sb/sbi structure of f2fs from all the >> parsing code, because with the new mount API the parsing is >> going to be done before we even get the super block. In this >> part, we introduce f2fs_fs_context to hold the temporary >> options when parsing. For the simple options check, it has >> to be done during parsing by using f2fs_fs_context structure. >> For the check which needs sb/sbi, we do this during super >> block filling. >> >> f2fs: Allow sbi to be NULL in f2fs_printk >> f2fs: Add f2fs_fs_context to record the mount options >> f2fs: separate the options parsing and options checking >> >> 3. Switch the f2fs to use the new mount API for mount and >> remount. >> >> f2fs: introduce fs_context_operation structure >> f2fs: switch to the new mount api >> >> 4. Cleanup the old unused structures and helpers. >> >> f2fs: remove unused structure and functions >> >> There is still a potential to do some cleanups and perhaps >> refactoring. However that can be done later after the conversion >> to the new mount API which is the main purpose of the patchset. >> >> [1] https://lore.kernel.org/all/20211021114508.21407-1-lczer...@redhat.com/ >> >> Hongbo Li (9): >> f2fs: Add fs parameter specifications for mount options >> f2fs: move the option parser into handle_mount_opt >> f2fs: move option validation into a separate helper >> f2fs: Allow sbi to be NULL in f2fs_printk >> f2fs: Add f2fs_fs_context to record the mount options >> f2fs: separate the options parsing and options checking >> f2fs: introduce fs_context_operation structure >> f2fs: switch to the new mount api >> f2fs: remove unused structure and functions >> >> fs/f2fs/super.c | 2211 --- >> 1 file changed, 1341 insertions(+), 870 deletions(-) >> > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/9] f2fs: Add fs parameter specifications for mount options
On 8/13/24 9:39 PM, Hongbo Li wrote:
> Use an array of `fs_parameter_spec` called f2fs_param_specs to
> hold the mount option specifications for the new mount api.
>
> Signed-off-by: Hongbo Li
> ---
> fs/f2fs/super.c | 79 +
> 1 file changed, 79 insertions(+)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 3959fd137cc9..1bd923a73c1f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include "f2fs.h"
> #include "node.h"
> @@ -189,9 +190,87 @@ enum {
> Opt_memory_mode,
> Opt_age_extent_cache,
> Opt_errors,
> + Opt_jqfmt,
> + Opt_checkpoint,
If adding an opt_jqfmt to use with an enum, you can/should remove
Opt_jqfmt_vfsold Opt_jqfmt_vfsv0, and Opt_jqfmt_vfsv1, because they
are no longer used.
Similarly for Opt_checkpoint_disable_* symbols.
> Opt_err,
> };
>
> +static const struct constant_table f2fs_param_jqfmt[] = {
> + {"vfsold", QFMT_VFS_OLD},
> + {"vfsv0", QFMT_VFS_V0},
> + {"vfsv1", QFMT_VFS_V1},
> + {}
> +};
> +
> +static const struct fs_parameter_spec f2fs_param_specs[] = {
> + fsparam_string("background_gc", Opt_gc_background),
> + fsparam_flag("disable_roll_forward", Opt_disable_roll_forward),
> + fsparam_flag("norecovery", Opt_norecovery),
Many/most other filesystems tab-align the param_spec, like
...
+ fsparam_string ("background_gc", Opt_gc_background),
+ fsparam_flag("disable_roll_forward",Opt_disable_roll_forward),
+ fsparam_flag("norecovery", Opt_norecovery),
...
but that's just a style thing, up to you and the maintainers.
I'd also suggest making more use of enums (as you did for f2fs_param_jqfmt).
I think it can simplify parsing in the long run if you choose to. It avoids
the "if strcmp() else if strcmp() else if strcmp()... pattern, for example
you can do:
static const struct constant_table f2fs_param_background_gc[] = {
{"on", BGGC_MODE_ON},
{"off", BGGC_MODE_OFF},
{"sync",BGGC_MODE_SYNC},
{}
};
...
fsparam_enum("background_gc", Opt_gc_background,
f2fs_param_background_gc),
...
and then parsing becomes simply:
case Opt_gc_background:
F2FS_CTX_INFO(ctx).bggc_mode = result.uint_32;
ctx->spec_mask |= F2FS_SPEC_background_gc;
break;
When I tried this I made a lot of use of enums, see
https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/tree/fs/f2fs/super.c?h=f2fs-mount-api#n182
and see what you think?
Thanks,
-Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2 20/23] xfs: add fs-verity support
On 4/4/23 11:27 AM, Darrick J. Wong wrote: > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index d40de32362b1..b6e99ed3b187 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -30,6 +30,7 @@ > #include "xfs_filestream.h" > #include "xfs_quota.h" > #include "xfs_sysfs.h" > +#include "xfs_verity.h" > #include "xfs_ondisk.h" > #include "xfs_rmap_item.h" > #include "xfs_refcount_item.h" > @@ -1489,6 +1490,9 @@ xfs_fs_fill_super( > sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ; > #endif > sb->s_op = &xfs_super_operations; > +#ifdef CONFIG_FS_VERITY > + sb->s_vop = &xfs_verity_ops; > +#endif > Hi Andrey - it might be nicer to just do: fsverity_set_ops(sb, &xfs_verity_ops); here and use the (existing) helper to avoid the #ifdef. (the #ifdef is handled by the helper) (ext4 & btrfs could use this too ...) Thanks! -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] xfstests: generic/342 run failed in f2fs
On 12/24/17 9:28 PM, Chen Rong wrote: > Hi, everyone: > > the issue as below: First we need to look - what does the test do? # Test that if we rename a file, create a new file that has the old name of the # other file and is a child of the same parent directory, fsync the new inode, # power fail and mount the filesystem, we do not lose the first file and that # file has the name it was renamed to. Ok, so it is a test of fsync'd file renames/creation over simulated device failure, and wants to be sure that all files and file contents are as expected if data integrity syscalls were made before the device failure. > # ./check generic/342 > FSTYP -- f2fs > PLATFORM -- Linux/x86_64 node01 4.15.0-rc4 > MKFS_OPTIONS -- /dev/sde1 > MOUNT_OPTIONS -- -o acl,user_xattr /dev/sde1 /tmp/test1 > > generic/342 - output mismatch (see > /home/ubuntu/git/xfstests-dev/results//generic/342.out.bad) > --- tests/generic/342.out 2017-12-13 13:47:20.14400 -0500 > +++ /home/ubuntu/git/xfstests-dev/results//generic/342.out.bad 2017-12-25 > 11:13:12.92800 -0500 > @@ -8,8 +8,7 @@ > 48c940ba3b8671d3d6ea74e4ccad8ca3 SCRATCH_MNT/a/bar > Directory a/ contents after log replay: > SCRATCH_MNT/a: > -bar the test created foo, fsynced it, then renamed it to bar and created a /new/ file also named foo, then fsynced the new file foo. Despite that 2nd fsync, the renamed file "bar" is not present in your case. However, don't we really need to fsync the directory after the rename and recreation to ensure persistence? iows: does this patch make the test pass on f2fs? It does for me, and I think it's the only valid way to run the test: diff --git a/tests/generic/342 b/tests/generic/342 index ed64e05..6a9e5bd 100755 --- a/tests/generic/342 +++ b/tests/generic/342 @@ -68,6 +68,7 @@ sync mv $SCRATCH_MNT/a/foo $SCRATCH_MNT/a/bar $XFS_IO_PROG -f -c "pwrite -S 0xba 0 16K" $SCRATCH_MNT/a/foo | _filter_xfs_io $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/a/foo +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/a echo "File digests before log replay:" md5sum $SCRATCH_MNT/a/foo | _filter_scratch without an fsync of the parent dir, I don't think we can guarantee that filename changes or additions will persist on every filesystem. > foo > File digests after log replay: > 9e5d56a1f9b2c93589f9d55480f971a1 SCRATCH_MNT/a/foo > ... > (Run 'diff -u tests/generic/342.out > /home/ubuntu/git/xfstests-dev/results//generic/342.out.bad' to see the > entire diff) Generally best to do that suggested diff command to see if there are any more pieces of changed/wrong test output. > Ran: generic/342 > Failures: generic/342 > Failed 1 of 1 tests > > Is it a problem with my computer or a already known issue with f2fs? or a problem with the test itself ... -Eric > thanks! > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] xfstests: generic/342 run failed in f2fs
On 12/24/17 10:30 PM, Chen Rong wrote: > > > On 2017年12月25日 13:56, Eric Sandeen wrote: >> On 12/24/17 9:28 PM, Chen Rong wrote: >>> Hi, everyone: >>> >>> the issue as below: >> First we need to look - what does the test do? >> >> # Test that if we rename a file, create a new file that has the old name of >> the >> # other file and is a child of the same parent directory, fsync the new >> inode, >> # power fail and mount the filesystem, we do not lose the first file and that >> # file has the name it was renamed to. >> >> Ok, so it is a test of fsync'd file renames/creation over simulated device >> failure, and wants to be sure that all files and file contents are as >> expected if data integrity syscalls were made before the device failure. >> >>> # ./check generic/342 >>> FSTYP -- f2fs >>> PLATFORM -- Linux/x86_64 node01 4.15.0-rc4 >>> MKFS_OPTIONS -- /dev/sde1 >>> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sde1 /tmp/test1 >>> >>> generic/342 - output mismatch (see >>> /home/ubuntu/git/xfstests-dev/results//generic/342.out.bad) >>> --- tests/generic/342.out 2017-12-13 13:47:20.14400 -0500 >>> +++ /home/ubuntu/git/xfstests-dev/results//generic/342.out.bad >>> 2017-12-25 11:13:12.92800 -0500 >>> @@ -8,8 +8,7 @@ >>> 48c940ba3b8671d3d6ea74e4ccad8ca3 SCRATCH_MNT/a/bar >>> Directory a/ contents after log replay: >>> SCRATCH_MNT/a: >>> -bar >> the test created foo, fsynced it, then renamed it to bar and created a >> /new/ file also named foo, then fsynced the new file foo. >> >> Despite that 2nd fsync, the renamed file "bar" is not present in >> your case. >> >> However, don't we really need to fsync the directory after >> the rename and recreation to ensure persistence? >> >> iows: does this patch make the test pass on f2fs? It does >> for me, and I think it's the only valid way to run the test: > the patch works for me. but btrfs could pass the test without it, why so > different? Filesystems are free to do /more/ than the minimum required by posix - see ext4_sync_parent for example. Or xfs_finish_rename, for synchronous mounts: * If this is a synchronous mount, make sure that the rename transaction * goes to disk before returning to the user. */ if (tp->t_mountp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC)) xfs_trans_set_sync(tp); so behavior can be fs-dependent, or mount option dependent, etc. But to be portable, if an app wants directory changes to be persistent before proceeding, it must fsync the directory after making changes. I don't know about f2fs's design intent, whether it guarantees more than posix requires, but to guarantee that this test works on any posix fs, I think that directory fsync is needed. -Eric -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] xfstests: generic/342 run failed in f2fs
On 12/28/17 1:09 AM, Dave Chinner wrote:
...
> There's a whole lot more detail in the kernel commit 2be63d5ce929
> ("Btrfs: fix file loss on log replay after renaming a file and
> fsync") but my point is that we considered this a btrfs filesystem
> bug and so changing the test defeats it's purpose as a regression
> test for the btrfs bug.
>
> So IMO the test should not be changed. And I think we should be
> consistent and consider this f2fs failure as a f2fs bug that needs
> fixing to bring it's behaviour in line with xfs, ext4, and btrfs.
>
> Remember this when quoting POSIX about fsync behaviour: Posix is a
> terrible standard when it comes to data integrity. We go way, way
> beyond what POSIX specifies as a valid fsync implementation (i.e.
> posix allows "do nothing and return success" as a conformant
> implementation). Ext4, XFS and btrfs all have strictly ordered
> metadata crash recovery semantics and all of the crash recovery
> tests expect this behaviour from the filesytem being tested. The
> underlying intention is that by encoding it into these tests, all
> widely used and future linux filesystems meet or exceed this crash
> integrity requirement.
>
> IOWs, changing the test is the wrong thing to do on many levels
>
> Cheers,
>
> Dave.
>
Thanks for digging into the btrfs commit which changed the behavior
tested by this testcase, I had meant to do that.
Yeah, that's all fair, for sure. But this shouldn't be implicit in the
testcase; it should explicitly document that it is testing a de-facto
new standard adhered to by several filesystems, what that standard is,
etc, and not leave it to the reader. ;)
If a filesystem explicitly chooses not to adhere to that standard they
/could/ opt out of the test.
It's makes a lot of sense for linux filesystems to behave in consistent
ways, but if we're going to start enforcing that through xfstests we
need to be very clear about it, IMHO.
-Eric
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] doc: cgroup: add f2fs and xfs to supported list for writeback
f2fs and xfs have both added support for cgroup writeback: 578c647 f2fs: implement cgroup writeback support adfb5fb xfs: implement cgroup aware writeback so add them to the supported list in the docs. Signed-off-by: Eric Sandeen --- TBH I wonder about the wisdom of having this detail in the doc, as it apparently gets missed quite often ... diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index ce3e05e..4f82afa 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1684,9 +1684,9 @@ per-cgroup dirty memory states are examined and the more restrictive of the two is enforced. cgroup writeback requires explicit support from the underlying -filesystem. Currently, cgroup writeback is implemented on ext2, ext4 -and btrfs. On other filesystems, all writeback IOs are attributed to -the root cgroup. +filesystem. Currently, cgroup writeback is implemented on ext2, ext4, +btrfs, f2fs, and xfs. On other filesystems, all writeback IOs are +attributed to the root cgroup. There are inherent differences in memory and writeback management which affects how cgroup ownership is tracked. Memory is tracked per ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] doc: cgroup: add f2fs and xfs to supported list for writeback
On 6/30/20 12:42 AM, Christoph Hellwig wrote: > On Mon, Jun 29, 2020 at 02:08:09PM -0500, Eric Sandeen wrote: >> f2fs and xfs have both added support for cgroup writeback: >> >> 578c647 f2fs: implement cgroup writeback support >> adfb5fb xfs: implement cgroup aware writeback >> >> so add them to the supported list in the docs. >> >> Signed-off-by: Eric Sandeen >> --- >> >> TBH I wonder about the wisdom of having this detail in >> the doc, as it apparently gets missed quite often ... > > I'd rather remove the list of file systems. It has no chance of > staying uptodate. Is there any way for a user to know whether a filesytem does or doesn't support it, in practice? Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] doc: cgroup: add f2fs and xfs to supported list for writeback
On 7/1/20 3:32 AM, Christoph Hellwig wrote: > On Tue, Jun 30, 2020 at 08:59:34AM -0500, Eric Sandeen wrote: >> On 6/30/20 12:42 AM, Christoph Hellwig wrote: >>> On Mon, Jun 29, 2020 at 02:08:09PM -0500, Eric Sandeen wrote: >>>> f2fs and xfs have both added support for cgroup writeback: >>>> >>>> 578c647 f2fs: implement cgroup writeback support >>>> adfb5fb xfs: implement cgroup aware writeback >>>> >>>> so add them to the supported list in the docs. >>>> >>>> Signed-off-by: Eric Sandeen >>>> --- >>>> >>>> TBH I wonder about the wisdom of having this detail in >>>> the doc, as it apparently gets missed quite often ... >>> >>> I'd rather remove the list of file systems. It has no chance of >>> staying uptodate. >> >> Is there any way for a user to know whether a filesytem does or doesn't >> support it, in practice? > > git-grep SB_I_CGROUPWB Sure, but that's not quite what I meant by "a user" :) So I'll take that as a no. Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/2] src/godown: support f2fs triggering specific ioctl
On 1/8/15 12:31 PM, Jaegeuk Kim wrote:
> This patch triggers the F2FS-related ioctl for godown.
hohum, wouldn't it be a whole lot easier to just re-use the XFS ioctl
number in f2fs? Then you wouldn't have to duplicate all this code.
If you really want your own unique ioctl number, what about
just doing statfs on the target, and if f_type returns F2FS_SUPER_MAGIC,
swictch the ioctl nr to F2FS_IOC_GOINGDOWN.
Then if not F2FS_SUPER_MAGIC, leave the ioctl nr at XFS_IOC_GOINGDOWN, and
if it fails, it fails (other filesystems might support the original nr.)
And then you can probably just add "f2fs" to the existing testcase,
and move it to tests/shared?
-Eric
> Signed-off-by: Jaegeuk Kim
> ---
> src/godown.c | 88
>
> 1 file changed, 65 insertions(+), 23 deletions(-)
>
> diff --git a/src/godown.c b/src/godown.c
> index b140a41..b44790b 100644
> --- a/src/godown.c
> +++ b/src/godown.c
> @@ -19,33 +19,82 @@
> #include
> #include "global.h"
>
> +#define F2FS_IOCTL_MAGIC 0xf5
> +#define F2FS_IOC_GOINGDOWN _IO(F2FS_IOCTL_MAGIC, 6)
> +
> +enum ftypes {
> + XFS_FS,
> + F2FS_FS,
> +};
> +
> static char *xprogname;
> +static char *mnt_dir;
> +static int verbose_opt = 0;
> +static int flushlog_opt = 0;
>
> +static enum ftypes fs = XFS_FS;
>
> static void
> usage(void)
> {
> - fprintf(stderr, "usage: %s [-f] [-v] mnt-dir\n", xprogname);
> + fprintf(stderr, "usage: %s [-f] [-v] [-s 0/1] mnt-dir\n", xprogname);
> +}
> +
> +static int
> +xfs_goingdown(int fd)
> +{
> + int flag;
> +
> + flag = (flushlog_opt ? XFS_FSOP_GOING_FLAGS_LOGFLUSH
> + : XFS_FSOP_GOING_FLAGS_NOLOGFLUSH);
> + if (verbose_opt) {
> + printf("Calling XFS_IOC_GOINGDOWN\n");
> + }
> +
> + syslog(LOG_WARNING, "xfstests-induced forced shutdown of %s:\n",
> + mnt_dir);
> + if ((xfsctl(mnt_dir, fd, XFS_IOC_GOINGDOWN, &flag)) == -1) {
> + fprintf(stderr, "%s: error on xfsctl(GOINGDOWN) of \"%s\":
> %s\n",
> + xprogname, mnt_dir, strerror(errno));
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int
> +f2fs_goingdown(int fd)
> +{
> + if (verbose_opt) {
> + printf("Calling F2FS_IOC_GOINGDOWN\n");
> + }
> + syslog(LOG_WARNING, "xfstests-induced forced shutdown of %s:\n",
> + mnt_dir);
> + if ((ioctl(fd, F2FS_IOC_GOINGDOWN)) == -1) {
> + fprintf(stderr, "%s: error on ioctl(GOINGDOWN) of \"%s\": %s\n",
> + xprogname, mnt_dir, strerror(errno));
> + return 1;
> + }
> + return 0;
> +
> }
>
> int
> main(int argc, char *argv[])
> {
> - int c;
> - int flag;
> - int flushlog_opt = 0;
> - int verbose_opt = 0;
> + int c, fd;
> struct stat st;
> - char *mnt_dir;
> - int fd;
> + int ret = 0;
>
> xprogname = argv[0];
>
> - while ((c = getopt(argc, argv, "fv")) != -1) {
> + while ((c = getopt(argc, argv, "fs:v")) != -1) {
> switch (c) {
> case 'f':
> flushlog_opt = 1;
> break;
> + case 's':
> + fs = atoi(optarg);
> + break;
> case 'v':
> verbose_opt = 1;
> break;
> @@ -94,10 +143,6 @@ main(int argc, char *argv[])
> }
> #endif
>
> -
> - flag = (flushlog_opt ? XFS_FSOP_GOING_FLAGS_LOGFLUSH
> - : XFS_FSOP_GOING_FLAGS_NOLOGFLUSH);
> -
> if (verbose_opt) {
> printf("Opening \"%s\"\n", mnt_dir);
> }
> @@ -107,18 +152,15 @@ main(int argc, char *argv[])
> return 1;
> }
>
> - if (verbose_opt) {
> - printf("Calling XFS_IOC_GOINGDOWN\n");
> + switch (fs) {
> + case XFS_FS:
> + ret = xfs_goingdown(fd);
> + break;
> + case F2FS_FS:
> + ret = f2fs_goingdown(fd);
> + break;
> }
> - syslog(LOG_WARNING, "xfstests-induced forced shutdown of %s:\n",
> - mnt_dir);
> - if ((xfsctl(mnt_dir, fd, XFS_IOC_GOINGDOWN, &flag)) == -1) {
> - fprintf(stderr, "%s: error on xfsctl(GOINGDOWN) of \"%s\":
> %s\n",
> - xprogname, mnt_dir, strerror(errno));
> - return 1;
> - }
> -
> close(fd);
>
> - return 0;
> + return ret;
> }
>
--
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
Re: [f2fs-dev] [PATCH 2/6] f2fs: support goingdown for fs shutdown
On 1/8/15 12:10 PM, Jaegeuk Kim wrote:
> This patch add an ioctl to shutdown f2fs, which stops all the further block
> writes after this point.
would it make sense to just re-use the xfs ioctl nr, if the semantics are
the same?
That way any test using it will "just work" on f2fs...
-Eric
> Signed-off-by: Jaegeuk Kim
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/file.c | 14 ++
> 2 files changed, 15 insertions(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ba30218..febad35 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -209,6 +209,7 @@ static inline bool __has_cursum_space(struct
> f2fs_summary_block *sum, int size,
> #define F2FS_IOC_START_VOLATILE_WRITE_IO(F2FS_IOCTL_MAGIC, 3)
> #define F2FS_IOC_RELEASE_VOLATILE_WRITE _IO(F2FS_IOCTL_MAGIC, 4)
> #define F2FS_IOC_ABORT_VOLATILE_WRITE_IO(F2FS_IOCTL_MAGIC, 5)
> +#define F2FS_IOC_GOINGDOWN _IO(F2FS_IOCTL_MAGIC, 6)
>
> #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
> /*
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 5df3367..de2f669 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1020,6 +1020,18 @@ static int f2fs_ioc_abort_volatile_write(struct file
> *filp)
> return ret;
> }
>
> +static int f2fs_ioc_goingdown(struct file *filp)
> +{
> + struct inode *inode = file_inode(filp);
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + f2fs_stop_checkpoint(sbi);
> + return 0;
> +}
> +
> static int f2fs_ioc_fitrim(struct file *filp, unsigned long arg)
> {
> struct inode *inode = file_inode(filp);
> @@ -1067,6 +1079,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> return f2fs_ioc_release_volatile_write(filp);
> case F2FS_IOC_ABORT_VOLATILE_WRITE:
> return f2fs_ioc_abort_volatile_write(filp);
> + case F2FS_IOC_GOINGDOWN:
> + return f2fs_ioc_goingdown(filp);
> case FITRIM:
> return f2fs_ioc_fitrim(filp, arg);
> default:
>
--
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/6] f2fs: support goingdown for fs shutdown
On 1/8/15 2:18 PM, Jaegeuk Kim wrote: > On Thu, Jan 08, 2015 at 01:54:20PM -0600, Eric Sandeen wrote: >> On 1/8/15 12:10 PM, Jaegeuk Kim wrote: >>> This patch add an ioctl to shutdown f2fs, which stops all the further block >>> writes after this point. >> >> would it make sense to just re-use the xfs ioctl nr, if the semantics are >> the same? > > The semantics are not same for now. > In order to reuse xfs ioctl, it needs to support options for flushing logs. the xfs iotl has 3 behaviors optional: #define XFS_FSOP_GOING_FLAGS_DEFAULT0x0 /* going down */ #define XFS_FSOP_GOING_FLAGS_LOGFLUSH 0x1 /* flush log but not data */ #define XFS_FSOP_GOING_FLAGS_NOLOGFLUSH 0x2 /* don't flush log nor data */ if f2fs currently supports a subset, you could just -EOPNOTSUPP on the others. If the semantics are completely different, maybe it shouldn't share the name at all. ;) Just a thought... -Eric > Thanks, > >> >> That way any test using it will "just work" on f2fs... >> >> -Eric -- Dive into the World of Parallel Programming! The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/2] generic/065: f2fs serves 64KB size with zero data
On 2/26/15 7:23 PM, Jaegeuk Kim wrote: > The f2fs provides 64KB size with 0 data after fsync was done to directory > file. > > Cc: Filipe Manana > Signed-off-by: Jaegeuk Kim > --- > tests/generic/065 | 4 > 1 file changed, 4 insertions(+) > > diff --git a/tests/generic/065 b/tests/generic/065 > index b5a296d..3d2b437 100755 > --- a/tests/generic/065 > +++ b/tests/generic/065 > @@ -139,6 +139,10 @@ ext3) > # a 64Kb file, with all bytes having the value 0xff > [ $hello_digest == "ecb99e6ffea7be1e5419350f725da86b" ] && digest_ok=yes > ;; > +f2fs) > + # a 64Kb file, with all bytes having the value 0 > + [ $hello_digest == "fcd6bcb56c1689fcef28b57c22475bad" ] && digest_ok=yes > + ;; whoa... I will admit to having poorly reviewed this test. Given that this file was never fsynced, I don't think the test should be looking at file contents *at all* I'll do an ex post facto review, I think, and really, I think all of the above should just be removed from the test. Without fsync, we don't know what's in the file. (ext3 could be mounted with writeback mode, for example). -Eric > *) > # an empty file > [ $hello_digest == "d41d8cd98f00b204e9800998ecf8427e" ] && digest_ok=yes > -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] Revert "f2fs: remove unreachable lazytime mount option parsing"
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: sta...@vger.kernel.org # 6.11+
>>> Signed-off-by: Daniel Rosenberg
>>> Signed-off-by: Jaegeuk Kim
>>> ---
>>> 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
>>
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] Revert "f2fs: remove unreachable lazytime mount option parsing"
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: sta...@vger.kernel.org # 6.11+
>> Signed-off-by: Daniel Rosenberg
>> Signed-off-by: Jaegeuk Kim
>> ---
>> 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
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] Revert "f2fs: remove unreachable lazytime mount option parsing"
On 11/20/24 2:38 PM, Jaegeuk Kim wrote:
> On 11/20, Eric Sandeen wrote:
...
>> (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.
(cc: list trimmed)
I had thought the conversion would resolve this too, but had not considered
direct
mount(2) calls passing the string in, which is something that probably needs to
be
supported even after the conversion, sadly.
As a reminder, this might be a start / sketch of how to convert to the new
mount API:
https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/log/?h=f2fs-mount-api
It's not entirely correct, but at least the first several patches are probably
the right
idea - getting sb / sbi out of the parsing path, and deferring
option-vs-disk-feature
checks until after the superblock is read, etc.
The final patch is probably not the way to go - it allocates an entire
f2fs_sb_info
in f2fs_init_fs_context - it probably makes more sense to create a new context
structure which holds only mount options, which is then transferred into the
sbi after option parsing during mount or remount.
I was doing these conversions as a side project, and given the f2fs conversion
complexity, I have yet to get to a series that I'm happy with. Perhaps expert
eyes can help!
Thanks,
-Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 7/9] f2fs: defer readonly check vs norecovery
Defer the readonly-vs-norecovery check until after option parsing is done
so that option parsing does not require an active superblock for the test.
Add a helpful message, while we're at it.
(I think could be moved back into parsing after we switch to the new mount
API if desired, as the fs context will have RO state available.)
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8866a74ce6aa..bc1aab749689 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -727,10 +727,8 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
set_opt(sbi, DISABLE_ROLL_FORWARD);
break;
case Opt_norecovery:
- /* this option mounts f2fs with ro */
+ /* requires ro mount, checked in f2fs_default_check */
set_opt(sbi, NORECOVERY);
- if (!f2fs_readonly(sb))
- return -EINVAL;
break;
case Opt_discard:
if (!f2fs_hw_support_discard(sbi)) {
@@ -1411,6 +1409,12 @@ static int f2fs_default_check(struct f2fs_sb_info *sbi)
f2fs_err(sbi, "Allow to mount readonly mode only");
return -EROFS;
}
+
+ if (test_opt(sbi, NORECOVERY) && !f2fs_readonly(sbi->sb)) {
+ f2fs_err(sbi, "norecovery requires readonly mount");
+ return -EINVAL;
+ }
+
return 0;
}
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion
I have been struggling to get to a good series to convert f2fs to the new mount API. f2fs is more complex, because much of the option parsing assumes that the superblock has already been read from disk, and uses that to test various on-disk features, etc. All of those tests will need to be moved to after parsing is complete, and this series is just a start. The first two patches in this series are incidental, just things I noticed when working on this. They are not critical to the conversion, but they may be desirable anyway. The rest of the patches move towards removal of explicit references to *sb in parse_options(), using *sbi instead. (The full conversion may use a private context structure instead of *sbi, since the *sbi is rather large.) It's up to you if you want to merge these now or not, but I thought I'd share the direction I was moving, to get some feedback about whether this seems to make sense. Next steps would be moving more of the feature checks to later in the mount process, after parsing is complete. This has been tested with random combinations of valid and invalid mount options, but it has not been tested with a wide range of on-disk features. My testing did not turn up any differences in behavior. (I also did explicit testing of direct mount syscalls with "lazytime" as an option string, keeping in mind the earlier regression there.) Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 6/9] f2fs: Pass sbi rather than sb to f2fs_set_test_dummy_encryption
This removes another sb instance from parse_options()
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e63b3bd75f85..8866a74ce6aa 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -483,12 +483,11 @@ static int f2fs_check_quota_options(struct f2fs_sb_info
*sbi)
}
#endif
-static int f2fs_set_test_dummy_encryption(struct super_block *sb,
+static int f2fs_set_test_dummy_encryption(struct f2fs_sb_info *sbi,
const char *opt,
const substring_t *arg,
bool is_remount)
{
- struct f2fs_sb_info *sbi = F2FS_SB(sb);
struct fs_parameter param = {
.type = fs_value_is_string,
.string = arg->from ? arg->from : "",
@@ -1029,7 +1028,7 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
kfree(name);
break;
case Opt_test_dummy_encryption:
- ret = f2fs_set_test_dummy_encryption(sb, p, &args[0],
+ ret = f2fs_set_test_dummy_encryption(sbi, p, &args[0],
is_remount);
if (ret)
return ret;
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function
From: Eric Sandeen
The current options parsing function both parses options and validates
them - factor the validation out to reduce the size of the function and
make transition to the new mount API possible, because under the new mount
API, options are parsed one at a time, and cannot all be tested at the end
of the parsing function.
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 29b3aa1ee99c..7cfd5e4e806e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
int ret;
if (!options)
- goto default_check;
+ return 0;
while ((p = strsep(&options, ",")) != NULL) {
int token;
@@ -1318,7 +1318,11 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
return -EINVAL;
}
}
-default_check:
+ return 0;
+}
+
+static int f2fs_default_check(struct f2fs_sb_info *sbi)
+{
#ifdef CONFIG_QUOTA
if (f2fs_check_quota_options(sbi))
return -EINVAL;
@@ -2364,6 +2368,10 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
}
#endif
+ err = f2fs_default_check(sbi);
+ if (err)
+ goto restore_opts;
+
/* flush outstanding errors before changing fs state */
flush_work(&sbi->s_error_work);
@@ -4489,6 +4497,10 @@ static int f2fs_fill_super(struct super_block *sb, void
*data, int silent)
if (err)
goto free_options;
+ err = f2fs_default_check(sbi);
+ if (err)
+ goto free_options;
+
sb->s_maxbytes = max_file_blocks(NULL) <<
le32_to_cpu(raw_super->log_blocksize);
sb->s_max_links = F2FS_LINK_MAX;
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 9/9] f2fs: pass sbi rather than sb to parse_options()
With the new mount API the sb will not be available during initial option
parsing, which will happen before fill_super reads sb from disk.
Now that the sb is no longer directly referenced in parse_options, switch
it to use sbi.
(Note that all calls to f2fs_sb_has_* originating from parse_options will
need to be deferred to later before we can use the new mount API.)
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 9edb200caae7..579c96a80fe2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -670,9 +670,8 @@ static int f2fs_set_zstd_level(struct f2fs_sb_info *sbi,
const char *str)
#endif
#endif
-static int parse_options(struct super_block *sb, char *options, bool
is_remount)
+static int parse_options(struct f2fs_sb_info *sbi, char *options, bool
is_remount)
{
- struct f2fs_sb_info *sbi = F2FS_SB(sb);
substring_t args[MAX_OPT_ARGS];
#ifdef CONFIG_F2FS_FS_COMPRESSION
unsigned char (*ext)[F2FS_EXTENSION_LEN];
@@ -2356,7 +2355,7 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
default_options(sbi, true);
/* parse mount options */
- err = parse_options(sb, data, true);
+ err = parse_options(sbi, data, true);
if (err)
goto restore_opts;
@@ -4496,7 +4495,7 @@ static int f2fs_fill_super(struct super_block *sb, void
*data, int silent)
goto free_sb_buf;
}
- err = parse_options(sb, options, false);
+ err = parse_options(sbi, options, false);
if (err)
goto free_options;
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 5/9] f2fs: make LAZYTIME a mount option flag
Set LAZYTIME into sbi during parsing, and transfer it to the sb in fill_super, so that an sb is not required during option parsing. (Note: While lazytime is normally handled via mount flag in the vfs, some f2fs users do expect to be able to use it as an explicit mount option string via the mount syscall, so this option must remain.) Signed-off-by: Eric Sandeen --- fs/f2fs/f2fs.h | 5 + fs/f2fs/super.c | 11 --- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 15e4f5a77eb5..5c83e3a558f9 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -115,6 +115,11 @@ extern const char *f2fs_fault_name[FAULT_MAX]; #define F2FS_MOUNT_COMPRESS_CACHE 0x0400 #define F2FS_MOUNT_AGE_EXTENT_CACHE0x0800 #define F2FS_MOUNT_INLINECRYPT 0x1000 +/* + * Some f2fs environments expect to be able to pass the "lazytime" option + * string rather than using the MS_LAZYTIME flag, so this must remain. + */ +#define F2FS_MOUNT_LAZYTIME0x2000 #define F2FS_OPTION(sbi) ((sbi)->mount_opt) #define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 643d19bbc156..e63b3bd75f85 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -917,10 +917,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) break; #endif case Opt_lazytime: - sb->s_flags |= SB_LAZYTIME; + set_opt(sbi, LAZYTIME); break; case Opt_nolazytime: - sb->s_flags &= ~SB_LAZYTIME; + clear_opt(sbi, LAZYTIME); break; #ifdef CONFIG_QUOTA case Opt_quota: @@ -2169,8 +2169,8 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount) set_opt(sbi, INLINE_DATA); set_opt(sbi, INLINE_DENTRY); set_opt(sbi, MERGE_CHECKPOINT); + set_opt(sbi, LAZYTIME); F2FS_OPTION(sbi).unusable_cap = 0; - sbi->sb->s_flags |= SB_LAZYTIME; if (!f2fs_is_readonly(sbi)) set_opt(sbi, FLUSH_MERGE); if (f2fs_sb_has_blkzoned(sbi)) @@ -4538,6 +4538,11 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) if (test_opt(sbi, INLINECRYPT)) sb->s_flags |= SB_INLINECRYPT; + if (test_opt(sbi, LAZYTIME)) + sb->s_flags |= SB_LAZYTIME; + else + sb->s_flags &= ~SB_LAZYTIME; + super_set_uuid(sb, (void *) raw_super->uuid, sizeof(raw_super->uuid)); super_set_sysfs_name_bdev(sb); sb->s_iflags |= SB_I_CGROUPWB; -- 2.48.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function
On 3/11/25 10:10 PM, Chao Yu wrote: > On 3/4/25 01:12, Eric Sandeen wrote: >> From: Eric Sandeen >> >> The current options parsing function both parses options and validates >> them - factor the validation out to reduce the size of the function and >> make transition to the new mount API possible, because under the new mount >> API, options are parsed one at a time, and cannot all be tested at the end >> of the parsing function. >> >> Signed-off-by: Eric Sandeen >> --- >> fs/f2fs/super.c | 16 ++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index 29b3aa1ee99c..7cfd5e4e806e 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char >> *options, bool is_remount) >> int ret; >> >> if (!options) >> -goto default_check; > > Eric, do you know in which condition options can be NULL, mount w/o any > specified options? > > If so, maybe we'd keep this in order to chech whether default options > generated from default_options() is conflicted or not? What do you think? > > Thanks, Yes, that's I think that is correct - (!options) is true when no f2fs-specific options are present for parsing. However, I think that we do still check default options with my patch in this case, because both calls to parse_options() still call f2fs_default_check() when parse_options() completes. Or am I misunderstanding your question? I added printks to check: # mount -o loop,ro f2fsfile.img mnt [root@fedora-rawhide f2fs-test]# dmesg [847946.326384] loop2: detected capacity change from 0 to 819200 [847946.337625] parse_options: (!options) is true [847946.337637] enter f2fs_default_check() Thank you for reviewing this series. I think at least the first 2 or 3 patches are suitable for merge now, if you agree. I am fairly certain that the rest of them are proper steps towards mount API conversion as well, but as I have not yet finished the work, I can't guarantee it yet. :) Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 4/9] f2fs: make INLINECRYPT a mount option flag
From: Eric Sandeen Set INLINECRYPT into sbi during parsing, and transfer it to the sb in fill_super, so that an sb is not required during option parsing. Signed-off-by: Eric Sandeen --- fs/f2fs/f2fs.h | 1 + fs/f2fs/super.c | 5 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 1afa7be16e7d..15e4f5a77eb5 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -114,6 +114,7 @@ extern const char *f2fs_fault_name[FAULT_MAX]; #defineF2FS_MOUNT_GC_MERGE 0x0200 #define F2FS_MOUNT_COMPRESS_CACHE 0x0400 #define F2FS_MOUNT_AGE_EXTENT_CACHE0x0800 +#define F2FS_MOUNT_INLINECRYPT 0x1000 #define F2FS_OPTION(sbi) ((sbi)->mount_opt) #define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 7cfd5e4e806e..643d19bbc156 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1036,7 +1036,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) break; case Opt_inlinecrypt: #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT - sb->s_flags |= SB_INLINECRYPT; + set_opt(sbi, INLINECRYPT); #else f2fs_info(sbi, "inline encryption not supported"); #endif @@ -4535,6 +4535,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) sb->s_time_gran = 1; sb->s_flags = (sb->s_flags & ~SB_POSIXACL) | (test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0); + if (test_opt(sbi, INLINECRYPT)) + sb->s_flags |= SB_INLINECRYPT; + super_set_uuid(sb, (void *) raw_super->uuid, sizeof(raw_super->uuid)); super_set_sysfs_name_bdev(sb); sb->s_iflags |= SB_I_CGROUPWB; -- 2.48.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 8/9] f2fs: pass sbi rather than sb to quota qf_name helpers
With the new mount api we will not have the superblock available during
option parsing. Prepare for this by passing *sbi rather than *sb.
For now, we are parsing after fill_super has been done, so sbi->sb will
exist. Under the new mount API this will require more care, but do the
simple change for now.
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bc1aab749689..9edb200caae7 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -383,10 +383,10 @@ static void init_once(void *foo)
#ifdef CONFIG_QUOTA
static const char * const quotatypes[] = INITQFNAMES;
#define QTYPE2NAME(t) (quotatypes[t])
-static int f2fs_set_qf_name(struct super_block *sb, int qtype,
+static int f2fs_set_qf_name(struct f2fs_sb_info *sbi, int qtype,
substring_t *args)
{
- struct f2fs_sb_info *sbi = F2FS_SB(sb);
+ struct super_block *sb = sbi->sb;
char *qname;
int ret = -EINVAL;
@@ -424,9 +424,9 @@ static int f2fs_set_qf_name(struct super_block *sb, int
qtype,
return ret;
}
-static int f2fs_clear_qf_name(struct super_block *sb, int qtype)
+static int f2fs_clear_qf_name(struct f2fs_sb_info *sbi, int qtype)
{
- struct f2fs_sb_info *sbi = F2FS_SB(sb);
+ struct super_block *sb = sbi->sb;
if (sb_any_quota_loaded(sb) && F2FS_OPTION(sbi).s_qf_names[qtype]) {
f2fs_err(sbi, "Cannot change journaled quota options when quota
turned on");
@@ -931,32 +931,32 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
set_opt(sbi, PRJQUOTA);
break;
case Opt_usrjquota:
- ret = f2fs_set_qf_name(sb, USRQUOTA, &args[0]);
+ ret = f2fs_set_qf_name(sbi, USRQUOTA, &args[0]);
if (ret)
return ret;
break;
case Opt_grpjquota:
- ret = f2fs_set_qf_name(sb, GRPQUOTA, &args[0]);
+ ret = f2fs_set_qf_name(sbi, GRPQUOTA, &args[0]);
if (ret)
return ret;
break;
case Opt_prjjquota:
- ret = f2fs_set_qf_name(sb, PRJQUOTA, &args[0]);
+ ret = f2fs_set_qf_name(sbi, PRJQUOTA, &args[0]);
if (ret)
return ret;
break;
case Opt_offusrjquota:
- ret = f2fs_clear_qf_name(sb, USRQUOTA);
+ ret = f2fs_clear_qf_name(sbi, USRQUOTA);
if (ret)
return ret;
break;
case Opt_offgrpjquota:
- ret = f2fs_clear_qf_name(sb, GRPQUOTA);
+ ret = f2fs_clear_qf_name(sbi, GRPQUOTA);
if (ret)
return ret;
break;
case Opt_offprjjquota:
- ret = f2fs_clear_qf_name(sb, PRJQUOTA);
+ ret = f2fs_clear_qf_name(sbi, PRJQUOTA);
if (ret)
return ret;
break;
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/9] f2fs: use f2fs_sb_has_device_alias during option parsing
Rather than using F2FS_HAS_FEATURE directly, use f2fs_sb_has_device_alias
macro during option parsing for consistency.
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 19b67828ae32..dd35d199775a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -838,7 +838,7 @@ static int parse_options(struct super_block *sb, char
*options, bool is_remount)
set_opt(sbi, READ_EXTENT_CACHE);
break;
case Opt_noextent_cache:
- if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_DEVICE_ALIAS)) {
+ if (f2fs_sb_has_device_alias(sbi)) {
f2fs_err(sbi, "device aliasing requires extent
cache");
return -EINVAL;
}
--
2.48.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/9] f2fs: consolidate unsupported option handling errors
When certain build-time options are disabled, some mount options are not accepted. For quota and compression, all related options are dismissed with a single error message. For xattr, acl, and fault injection, each option is handled individually. In addition, inline_xattr_size was missed when CONFIG_F2FS_FS_XATTR was disabled. Collapse xattr, acl, and fault injection errors into a single string, for simplicity, and handle the missing inline_xattr_size case. Signed-off-by: Eric Sandeen --- fs/f2fs/super.c | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index dd35d199775a..29b3aa1ee99c 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -772,16 +772,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) break; #else case Opt_user_xattr: - f2fs_info(sbi, "user_xattr options not supported"); - break; case Opt_nouser_xattr: - f2fs_info(sbi, "nouser_xattr options not supported"); - break; case Opt_inline_xattr: - f2fs_info(sbi, "inline_xattr options not supported"); - break; case Opt_noinline_xattr: - f2fs_info(sbi, "noinline_xattr options not supported"); + case Opt_inline_xattr_size: + f2fs_info(sbi, "xattr options not supported"); break; #endif #ifdef CONFIG_F2FS_FS_POSIX_ACL @@ -793,10 +788,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) break; #else case Opt_acl: - f2fs_info(sbi, "acl options not supported"); - break; case Opt_noacl: - f2fs_info(sbi, "noacl options not supported"); + f2fs_info(sbi, "acl options not supported"); break; #endif case Opt_active_logs: @@ -919,11 +912,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) break; #else case Opt_fault_injection: - f2fs_info(sbi, "fault_injection options not supported"); - break; - case Opt_fault_type: - f2fs_info(sbi, "fault_type options not supported"); + f2fs_info(sbi, "fault injection options not supported"); break; #endif case Opt_lazytime: -- 2.48.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion
I was working on next steps for this, and I have a followup question.
Today, several mount options are simply ignored if the on-disk format
does not support them. For example:
case Opt_compress_mode:
if (!f2fs_sb_has_compression(sbi)) {
f2fs_info(sbi, "Image doesn't support
compression");
break;
}
name = match_strdup(&args[0]);
if (!name)
return -ENOMEM;
if (!strcmp(name, "fs")) {
F2FS_OPTION(sbi).compress_mode = COMPR_MODE_FS;
} else if (!strcmp(name, "user")) {
F2FS_OPTION(sbi).compress_mode =
COMPR_MODE_USER;
} else {
kfree(name);
return -EINVAL;
}
kfree(name);
break;
so if f2fs_sb_has_compression() is not true, then the option is ignored without
any validation.
in other words, "mount -o compress_mode=nope ..." will succeed if the feature
is disabled on the filesystem.
If I move the f2fs_sb_has_compression() check to later for the new mount API,
then "mount -o compress_mode=nope ..." will start failing for all images. Is
this acceptable? It seems wise to reject invalid options rather than ignore
them,
even if they are incompatible with the format, but this would be a behavior
change.
The above would be simple enough to defer (maybe set to COMPR_MODE_INVAL and
reject it later) but I think other options such as compress/nocompress
extensions
would be very messy to approach as "accept all options given during parsing,
and validate them later only if the corresponding feature is present."
So I wonder if a behavior change (stricter option validation) would be
acceptable here?
Thanks,
-Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion
On 3/31/25 3:31 AM, Chao Yu wrote:
> On 3/29/25 12:18, Eric Sandeen wrote:
>> I was working on next steps for this, and I have a followup question.
>>
>> Today, several mount options are simply ignored if the on-disk format
>> does not support them. For example:
>>
>> case Opt_compress_mode:
>> if (!f2fs_sb_has_compression(sbi)) {
>> f2fs_info(sbi, "Image doesn't support
>> compression");
>> break;
>> }
>> name = match_strdup(&args[0]);
>> if (!name)
>> return -ENOMEM;
>> if (!strcmp(name, "fs")) {
>> F2FS_OPTION(sbi).compress_mode =
>> COMPR_MODE_FS;
>> } else if (!strcmp(name, "user")) {
>> F2FS_OPTION(sbi).compress_mode =
>> COMPR_MODE_USER;
>> } else {
>> kfree(name);
>> return -EINVAL;
>> }
>> kfree(name);
>> break;
>>
>> so if f2fs_sb_has_compression() is not true, then the option is ignored
>> without
>> any validation.
>>
>> in other words, "mount -o compress_mode=nope ..." will succeed if the feature
>> is disabled on the filesystem.
>>
>> If I move the f2fs_sb_has_compression() check to later for the new mount API,
>> then "mount -o compress_mode=nope ..." will start failing for all images. Is
>> this acceptable? It seems wise to reject invalid options rather than ignore
>> them,
>> even if they are incompatible with the format, but this would be a behavior
>> change.
>
> I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
> should stop passing invalid mount option to f2fs.
Great, I will proceed with this. It will make the conversion simpler (but may
make testing/validation more difficult, as behavior will change with invalid
input).
-Eric
> Thanks,
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 0/7 V2] f2fs: new mount API conversion
On 4/20/25 10:24 AM, Eric Sandeen wrote: > This is a forward-port of Hongbo's original f2fs mount API conversion, > posted last August at > https://lore.kernel.org/linux-f2fs-devel/20240814023912.3959299-1-lihongb...@huawei.com/ I'll rebase this onto jaegeuk's dev tree and send a V3. -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH V3 6/7] f2fs: introduce fs_context_operation structure
From: Hongbo Li
The handle_mount_opt() helper is used to parse mount parameters,
and so we can rename this function to f2fs_parse_param() and set
it as .param_param in fs_context_operations.
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 149134775870..37497fd80bb9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -707,7 +707,7 @@ static int f2fs_set_zstd_level(struct f2fs_fs_context *ctx,
const char *str)
#endif
#endif
-static int handle_mount_opt(struct fs_context *fc, struct fs_parameter *param)
+static int f2fs_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct f2fs_fs_context *ctx = fc->fs_private;
#ifdef CONFIG_F2FS_FS_COMPRESSION
@@ -1173,7 +1173,7 @@ static int parse_options(struct fs_context *fc, char
*options)
param.key = key;
param.size = v_len;
- ret = handle_mount_opt(fc, ¶m);
+ ret = f2fs_parse_param(fc, ¶m);
kfree(param.string);
if (ret < 0)
return ret;
@@ -5277,6 +5277,10 @@ static struct dentry *f2fs_mount(struct file_system_type
*fs_type, int flags,
return mount_bdev(fs_type, flags, dev_name, data, f2fs_fill_super);
}
+static const struct fs_context_operations f2fs_context_ops = {
+ .parse_param= f2fs_parse_param,
+};
+
static void kill_f2fs_super(struct super_block *sb)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
--
2.49.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH V3 3/7] f2fs: Allow sbi to be NULL in f2fs_printk
From: Hongbo Li
At the parsing phase of the new mount api, sbi will not be
available. So here allows sbi to be NULL in f2fs log helpers
and use that in handle_mount_opt().
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 90 +++--
1 file changed, 49 insertions(+), 41 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 20dee7c40d59..35190db4501c 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -323,11 +323,19 @@ void f2fs_printk(struct f2fs_sb_info *sbi, bool
limit_rate,
vaf.fmt = printk_skip_level(fmt);
vaf.va = &args;
if (limit_rate)
- printk_ratelimited("%c%cF2FS-fs (%s): %pV\n",
- KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ if (sbi)
+ printk_ratelimited("%c%cF2FS-fs (%s): %pV\n",
+ KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ else
+ printk_ratelimited("%c%cF2FS-fs: %pV\n",
+ KERN_SOH_ASCII, level, &vaf);
else
- printk("%c%cF2FS-fs (%s): %pV\n",
- KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ if (sbi)
+ printk("%c%cF2FS-fs (%s): %pV\n",
+ KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ else
+ printk("%c%cF2FS-fs: %pV\n",
+ KERN_SOH_ASCII, level, &vaf);
va_end(args);
}
@@ -737,13 +745,13 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_discard:
if (result.negated) {
if (f2fs_hw_should_discard(sbi)) {
- f2fs_warn(sbi, "discard is required for zoned
block devices");
+ f2fs_warn(NULL, "discard is required for zoned
block devices");
return -EINVAL;
}
clear_opt(sbi, DISCARD);
} else {
if (!f2fs_hw_support_discard(sbi)) {
- f2fs_warn(sbi, "device does not support
discard");
+ f2fs_warn(NULL, "device does not support
discard");
break;
}
set_opt(sbi, DISCARD);
@@ -751,7 +759,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
case Opt_noheap:
case Opt_heap:
- f2fs_warn(sbi, "heap/no_heap options were deprecated");
+ f2fs_warn(NULL, "heap/no_heap options were deprecated");
break;
#ifdef CONFIG_F2FS_FS_XATTR
case Opt_user_xattr:
@@ -774,7 +782,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_user_xattr:
case Opt_inline_xattr:
case Opt_inline_xattr_size:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -786,7 +794,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
#else
case Opt_acl:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
case Opt_active_logs:
@@ -840,7 +848,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
case Opt_reserve_root:
if (test_opt(sbi, RESERVE_ROOT)) {
- f2fs_info(sbi, "Preserve previous reserve_root=%u",
+ f2fs_info(NULL, "Preserve previous reserve_root=%u",
F2FS_OPTION(sbi).root_reserved_blocks);
} else {
F2FS_OPTION(sbi).root_reserved_blocks = result.int_32;
@@ -871,7 +879,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
#else
case Opt_fault_injection:
case Opt_fault_type:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
case Opt_lazytime:
@@ -934,7 +942,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_usrjquota:
case Opt_grpjquota:
[f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
V3: - Rebase onto git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev branch - Fix up some 0day robot warnings This is a forward-port of Hongbo's original f2fs mount API conversion, posted last August at https://lore.kernel.org/linux-f2fs-devel/20240814023912.3959299-1-lihongb...@huawei.com/ I had been trying to approach this with a little less complexity, but in the end I realized that Hongbo's approach (which follows the ext4 approach) was a good one, and I was not making any progrss myself. 😉 In addition to the forward-port, I have also fixed a couple bugs I found during testing, and some improvements / style choices as well. Hongbo and I have discussed most of this off-list already, so I'm presenting the net result here. This does pass my typical testing which does a large number of random mounts/remounts with valid and invalid option sets, on f2fs filesystem images with various features in the on-disk superblock. (I was not able to test all of this completely, as some options or features require hardware I dn't have.) Thanks, -Eric (A recap of Hongbo's original cover letter is below, edited slightly for this series:) Since many filesystems have done the new mount API conversion, we introduce the new mount API conversion in f2fs. The series can be applied on top of the current mainline tree and the work is based on the patches from Lukas Czerner (has done this in ext4[1]). His patch give me a lot of ideas. Here is a high level description of the patchset: 1. Prepare the f2fs mount parameters required by the new mount API and use it for parsing, while still using the old API to get mount options string. Split the parameter parsing and validation of the parse_options helper into two separate helpers. f2fs: Add fs parameter specifications for mount options f2fs: move the option parser into handle_mount_opt 2. Remove the use of sb/sbi structure of f2fs from all the parsing code, because with the new mount API the parsing is going to be done before we even get the super block. In this part, we introduce f2fs_fs_context to hold the temporary options when parsing. For the simple options check, it has to be done during parsing by using f2fs_fs_context structure. For the check which needs sb/sbi, we do this during super block filling. f2fs: Allow sbi to be NULL in f2fs_printk f2fs: Add f2fs_fs_context to record the mount options f2fs: separate the options parsing and options checking 3. Switch the f2fs to use the new mount API for mount and remount. f2fs: introduce fs_context_operation structure f2fs: switch to the new mount api [1] https://lore.kernel.org/all/20211021114508.21407-1-lczer...@redhat.com/ ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH V3 4/7] f2fs: Add f2fs_fs_context to record the mount options
From: Hongbo Li
At the parsing phase of mouont in the new mount api, options
value will be recorded with the context, and then it will be
used in fill_super and other helpers.
Note that, this is a temporary status, we want remove the sb
and sbi usages in handle_mount_opt. So here the f2fs_fs_context
only records the mount options, it will be copied in sb/sbi in
later process. (At this point in the series, mount options are
temporarily not set during mount.)
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 419
1 file changed, 244 insertions(+), 175 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 35190db4501c..15befeb45c94 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -310,8 +310,65 @@ static match_table_t f2fs_checkpoint_tokens = {
{Opt_err, NULL},
};
+#define F2FS_SPEC_background_gc(1 << 0)
+#define F2FS_SPEC_inline_xattr_size(1 << 1)
+#define F2FS_SPEC_active_logs (1 << 2)
+#define F2FS_SPEC_reserve_root (1 << 3)
+#define F2FS_SPEC_resgid (1 << 4)
+#define F2FS_SPEC_resuid (1 << 5)
+#define F2FS_SPEC_mode (1 << 6)
+#define F2FS_SPEC_fault_injection (1 << 7)
+#define F2FS_SPEC_fault_type (1 << 8)
+#define F2FS_SPEC_jqfmt(1 << 9)
+#define F2FS_SPEC_alloc_mode (1 << 10)
+#define F2FS_SPEC_fsync_mode (1 << 11)
+#define F2FS_SPEC_checkpoint_disable_cap (1 << 12)
+#define F2FS_SPEC_checkpoint_disable_cap_perc (1 << 13)
+#define F2FS_SPEC_compress_level (1 << 14)
+#define F2FS_SPEC_compress_algorithm (1 << 15)
+#define F2FS_SPEC_compress_log_size(1 << 16)
+#define F2FS_SPEC_compress_extension (1 << 17)
+#define F2FS_SPEC_nocompress_extension (1 << 18)
+#define F2FS_SPEC_compress_chksum (1 << 19)
+#define F2FS_SPEC_compress_mode(1 << 20)
+#define F2FS_SPEC_discard_unit (1 << 21)
+#define F2FS_SPEC_memory_mode (1 << 22)
+#define F2FS_SPEC_errors (1 << 23)
+
+struct f2fs_fs_context {
+ struct f2fs_mount_info info;
+ unsigned intopt_mask; /* Bits changed */
+ unsigned intspec_mask;
+ unsigned short qname_mask;
+ unsigned long sflags;
+ unsigned long sflags_mask;
+};
+
+#define F2FS_CTX_INFO(ctx) ((ctx)->info)
+
+static inline void ctx_set_opt(struct f2fs_fs_context *ctx,
+ unsigned int flag)
+{
+ ctx->info.opt |= flag;
+ ctx->opt_mask |= flag;
+}
+
+static inline void ctx_clear_opt(struct f2fs_fs_context *ctx,
+unsigned int flag)
+{
+ ctx->info.opt &= ~flag;
+ ctx->opt_mask |= flag;
+}
+
+static inline void ctx_set_flags(struct f2fs_fs_context *ctx,
+unsigned int flag)
+{
+ ctx->sflags |= flag;
+ ctx->sflags_mask |= flag;
+}
+
void f2fs_printk(struct f2fs_sb_info *sbi, bool limit_rate,
- const char *fmt, ...)
+ const char *fmt, ...)
{
struct va_format vaf;
va_list args;
@@ -429,57 +486,51 @@ static void init_once(void *foo)
#ifdef CONFIG_QUOTA
static const char * const quotatypes[] = INITQFNAMES;
#define QTYPE2NAME(t) (quotatypes[t])
-static int f2fs_set_qf_name(struct f2fs_sb_info *sbi, int qtype,
- struct fs_parameter *param)
+/*
+ * Note the name of the specified quota file.
+ */
+static int f2fs_note_qf_name(struct fs_context *fc, int qtype,
+struct fs_parameter *param)
{
- struct super_block *sb = sbi->sb;
+ struct f2fs_fs_context *ctx = fc->fs_private;
char *qname;
- int ret = -EINVAL;
- if (sb_any_quota_loaded(sb) && !F2FS_OPTION(sbi).s_qf_names[qtype]) {
- f2fs_err(sbi, "Cannot change journaled quota options when quota
turned on");
+ if (param->size < 1) {
+ f2fs_err(NULL, "Missing quota name");
return -EINVAL;
}
- if (f2fs_sb_has_quota_ino(sbi)) {
- f2fs_info(sbi, "QUOTA feature is enabled, so ignore qf_name");
+ if (strchr(param->string, '/')) {
+ f2fs_err(NULL, "quotafile must be on filesystem root");
+ return -EINVAL;
+ }
+ if (ctx->info.s_qf_names[qtype]) {
+ if (strcmp(ctx->info.s_qf_names[qtype
[f2fs-dev] [PATCH V3 5/7] f2fs: separate the options parsing and options checking
From: Hongbo Li
The new mount api separates option parsing and super block setup
into two distinct steps and so we need to separate the options
parsing out of the parse_options().
In order to achieve this, here we handle the mount options with
three steps:
- Firstly, we move sb/sbi out of handle_mount_opt.
As the former patch introduced f2fs_fs_context, so we record
the changed mount options in this context. In handle_mount_opt,
sb/sbi is null, so we should move all relative code out of
handle_mount_opt (thus, some check case which use sb/sbi should
move out).
- Secondly, we introduce the some check helpers to keep the option
consistent.
During filling superblock period, sb/sbi are ready. So we check
the f2fs_fs_context which holds the mount options base on sb/sbi.
- Thirdly, we apply the new mount options to sb/sbi.
After checking the f2fs_fs_context, all changed on mount options
are valid. So we can apply them to sb/sbi directly.
After do these, option parsing and super block setting have been
decoupled. Also it should have retained the original execution
flow.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 693 +++-
1 file changed, 510 insertions(+), 183 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 15befeb45c94..149134775870 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -360,6 +360,12 @@ static inline void ctx_clear_opt(struct f2fs_fs_context
*ctx,
ctx->opt_mask |= flag;
}
+static inline bool ctx_test_opt(struct f2fs_fs_context *ctx,
+ unsigned int flag)
+{
+ return ctx->info.opt & flag;
+}
+
static inline void ctx_set_flags(struct f2fs_fs_context *ctx,
unsigned int flag)
{
@@ -533,51 +539,6 @@ static int f2fs_unnote_qf_name(struct fs_context *fc, int
qtype)
ctx->qname_mask |= 1 << qtype;
return 0;
}
-
-static int f2fs_check_quota_options(struct f2fs_sb_info *sbi)
-{
- /*
-* We do the test below only for project quotas. 'usrquota' and
-* 'grpquota' mount options are allowed even without quota feature
-* to support legacy quotas in quota files.
-*/
- if (test_opt(sbi, PRJQUOTA) && !f2fs_sb_has_project_quota(sbi)) {
- f2fs_err(sbi, "Project quota feature not enabled. Cannot enable
project quota enforcement.");
- return -1;
- }
- if (F2FS_OPTION(sbi).s_qf_names[USRQUOTA] ||
- F2FS_OPTION(sbi).s_qf_names[GRPQUOTA] ||
- F2FS_OPTION(sbi).s_qf_names[PRJQUOTA]) {
- if (test_opt(sbi, USRQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[USRQUOTA])
- clear_opt(sbi, USRQUOTA);
-
- if (test_opt(sbi, GRPQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[GRPQUOTA])
- clear_opt(sbi, GRPQUOTA);
-
- if (test_opt(sbi, PRJQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[PRJQUOTA])
- clear_opt(sbi, PRJQUOTA);
-
- if (test_opt(sbi, GRPQUOTA) || test_opt(sbi, USRQUOTA) ||
- test_opt(sbi, PRJQUOTA)) {
- f2fs_err(sbi, "old and new quota format mixing");
- return -1;
- }
-
- if (!F2FS_OPTION(sbi).s_jquota_fmt) {
- f2fs_err(sbi, "journaled quota format not specified");
- return -1;
- }
- }
-
- if (f2fs_sb_has_quota_ino(sbi) && F2FS_OPTION(sbi).s_jquota_fmt) {
- f2fs_info(sbi, "QUOTA feature is enabled, so ignore
jquota_fmt");
- F2FS_OPTION(sbi).s_jquota_fmt = 0;
- }
- return 0;
-}
#endif
static int f2fs_parse_test_dummy_encryption(const struct fs_parameter *param,
@@ -636,28 +597,28 @@ static bool is_compress_extension_exist(struct
f2fs_mount_info *info,
* extension will be treated as special cases and will not be compressed.
* 3. Don't allow the non-compress extension specifies all files.
*/
-static int f2fs_test_compress_extension(struct f2fs_sb_info *sbi)
+static int f2fs_test_compress_extension(unsigned char
(*noext)[F2FS_EXTENSION_LEN],
+ int noext_cnt,
+ unsigned char
(*ext)[F2FS_EXTENSION_LEN],
+ int ext_cnt)
{
- unsigned char (*ext)[F2FS_EXTENSION_LEN];
- unsigned char (*noext)[F2FS_EXTENSION_LEN];
- int ext_cnt, noext_cnt, index = 0, no_index = 0;
-
- ext = F2FS_OPTION(sbi).extensions;
- ext_cnt = F2FS_OPTION(sbi).compress_ext_cn
[f2fs-dev] [PATCH V3 7/7] f2fs: switch to the new mount api
From: Hongbo Li
The new mount api will execute .parse_param, .init_fs_context, .get_tree
and will call .remount if remount happened. So we add the necessary
functions for the fs_context_operations. If .init_fs_context is added,
the old .mount should remove.
See Documentation/filesystems/mount_api.rst for more information.
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 156 +++-
1 file changed, 62 insertions(+), 94 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 37497fd80bb9..041bd6c482a0 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1141,47 +1141,6 @@ static int f2fs_parse_param(struct fs_context *fc,
struct fs_parameter *param)
return 0;
}
-static int parse_options(struct fs_context *fc, char *options)
-{
- struct fs_parameter param;
- char *key;
- int ret;
-
- if (!options)
- return 0;
-
- while ((key = strsep(&options, ",")) != NULL) {
- if (*key) {
- size_t v_len = 0;
- char *value = strchr(key, '=');
-
- param.type = fs_value_is_flag;
- param.string = NULL;
-
- if (value) {
- if (value == key)
- continue;
-
- *value++ = 0;
- v_len = strlen(value);
- param.string = kmemdup_nul(value, v_len,
GFP_KERNEL);
- if (!param.string)
- return -ENOMEM;
- param.type = fs_value_is_string;
- }
-
- param.key = key;
- param.size = v_len;
-
- ret = f2fs_parse_param(fc, ¶m);
- kfree(param.string);
- if (ret < 0)
- return ret;
- }
- }
- return 0;
-}
-
/*
* Check quota settings consistency.
*/
@@ -2583,13 +2542,12 @@ static void f2fs_enable_checkpoint(struct f2fs_sb_info
*sbi)
f2fs_flush_ckpt_thread(sbi);
}
-static int f2fs_remount(struct super_block *sb, int *flags, char *data)
+static int __f2fs_remount(struct fs_context *fc, struct super_block *sb)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
struct f2fs_mount_info org_mount_opt;
- struct f2fs_fs_context ctx;
- struct fs_context fc;
unsigned long old_sb_flags;
+ unsigned int flags = fc->sb_flags;
int err;
bool need_restart_gc = false, need_stop_gc = false;
bool need_restart_flush = false, need_stop_flush = false;
@@ -2635,7 +2593,7 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
#endif
/* recover superblocks we couldn't write due to previous RO mount */
- if (!(*flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
+ if (!(flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
err = f2fs_commit_super(sbi, false);
f2fs_info(sbi, "Try to recover all the superblocks, ret: %d",
err);
@@ -2645,21 +2603,11 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
default_options(sbi, true);
- memset(&fc, 0, sizeof(fc));
- memset(&ctx, 0, sizeof(ctx));
- fc.fs_private = &ctx;
- fc.purpose = FS_CONTEXT_FOR_RECONFIGURE;
-
- /* parse mount options */
- err = parse_options(&fc, data);
- if (err)
- goto restore_opts;
-
- err = f2fs_check_opt_consistency(&fc, sb);
+ err = f2fs_check_opt_consistency(fc, sb);
if (err < 0)
goto restore_opts;
- f2fs_apply_options(&fc, sb);
+ f2fs_apply_options(fc, sb);
#ifdef CONFIG_BLK_DEV_ZONED
if (f2fs_sb_has_blkzoned(sbi) &&
@@ -2678,20 +2626,20 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
* Previous and new state of filesystem is RO,
* so skip checking GC and FLUSH_MERGE conditions.
*/
- if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
+ if (f2fs_readonly(sb) && (flags & SB_RDONLY))
goto skip;
- if (f2fs_dev_is_readonly(sbi) && !(*flags & SB_RDONLY)) {
+ if (f2fs_dev_is_readonly(sbi) && !(flags & SB_RDONLY)) {
err = -EROFS;
goto restore_opts;
}
#ifdef CONFIG_QUOTA
- if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
+ if (!f2fs_readonly(sb) && (flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
if (
[f2fs-dev] [PATCH V3 1/7] f2fs: Add fs parameter specifications for mount options
From: Hongbo Li
Use an array of `fs_parameter_spec` called f2fs_param_specs to
hold the mount option specifications for the new mount api.
Add constant_table structures for several options to facilitate
parsing.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates, more fsparam_enum]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 122
1 file changed, 122 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 22f26871b7aa..ebea03bba054 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "f2fs.h"
#include "node.h"
@@ -194,9 +195,130 @@ enum {
Opt_age_extent_cache,
Opt_errors,
Opt_nat_bits,
+ Opt_jqfmt,
+ Opt_checkpoint,
Opt_err,
};
+static const struct constant_table f2fs_param_background_gc[] = {
+ {"on", BGGC_MODE_ON},
+ {"off", BGGC_MODE_OFF},
+ {"sync",BGGC_MODE_SYNC},
+ {}
+};
+
+static const struct constant_table f2fs_param_mode[] = {
+ {"adaptive",FS_MODE_ADAPTIVE},
+ {"lfs", FS_MODE_LFS},
+ {"fragment:segment",FS_MODE_FRAGMENT_SEG},
+ {"fragment:block", FS_MODE_FRAGMENT_BLK},
+ {}
+};
+
+static const struct constant_table f2fs_param_jqfmt[] = {
+ {"vfsold", QFMT_VFS_OLD},
+ {"vfsv0", QFMT_VFS_V0},
+ {"vfsv1", QFMT_VFS_V1},
+ {}
+};
+
+static const struct constant_table f2fs_param_alloc_mode[] = {
+ {"default", ALLOC_MODE_DEFAULT},
+ {"reuse", ALLOC_MODE_REUSE},
+ {}
+};
+static const struct constant_table f2fs_param_fsync_mode[] = {
+ {"posix", FSYNC_MODE_POSIX},
+ {"strict", FSYNC_MODE_STRICT},
+ {"nobarrier", FSYNC_MODE_NOBARRIER},
+ {}
+};
+
+static const struct constant_table f2fs_param_compress_mode[] = {
+ {"fs", COMPR_MODE_FS},
+ {"user",COMPR_MODE_USER},
+ {}
+};
+
+static const struct constant_table f2fs_param_discard_unit[] = {
+ {"block", DISCARD_UNIT_BLOCK},
+ {"segment", DISCARD_UNIT_SEGMENT},
+ {"section", DISCARD_UNIT_SECTION},
+ {}
+};
+
+static const struct constant_table f2fs_param_memory_mode[] = {
+ {"normal", MEMORY_MODE_NORMAL},
+ {"low", MEMORY_MODE_LOW},
+ {}
+};
+
+static const struct constant_table f2fs_param_errors[] = {
+ {"remount-ro", MOUNT_ERRORS_READONLY},
+ {"continue",MOUNT_ERRORS_CONTINUE},
+ {"panic", MOUNT_ERRORS_PANIC},
+ {}
+};
+
+static const struct fs_parameter_spec f2fs_param_specs[] = {
+ fsparam_enum("background_gc", Opt_gc_background,
f2fs_param_background_gc),
+ fsparam_flag("disable_roll_forward", Opt_disable_roll_forward),
+ fsparam_flag("norecovery", Opt_norecovery),
+ fsparam_flag_no("discard", Opt_discard),
+ fsparam_flag("no_heap", Opt_noheap),
+ fsparam_flag("heap", Opt_heap),
+ fsparam_flag_no("user_xattr", Opt_user_xattr),
+ fsparam_flag_no("acl", Opt_acl),
+ fsparam_s32("active_logs", Opt_active_logs),
+ fsparam_flag("disable_ext_identify", Opt_disable_ext_identify),
+ fsparam_flag_no("inline_xattr", Opt_inline_xattr),
+ fsparam_s32("inline_xattr_size", Opt_inline_xattr_size),
+ fsparam_flag_no("inline_data", Opt_inline_data),
+ fsparam_flag_no("inline_dentry", Opt_inline_dentry),
+ fsparam_flag_no("flush_merge", Opt_flush_merge),
+ fsparam_flag_no("barrier", Opt_barrier),
+ fsparam_flag("fastboot", Opt_fastboot),
+ fsparam_flag_no("extent_cache", Opt_extent_cache),
+ fsparam_flag("data_flush", Opt_data_flush),
+ fsparam_u32("reserve_root", Opt_reserve_root),
+ fsparam_gid("resgid", Opt_resgid),
+ fsparam_uid("resuid", Opt_resuid),
+ fsparam_enum("mode", Opt_mode, f2fs_param_mode),
+ fsparam_s32("fault_injection", Opt_fault_injection),
+ fsparam_u32("fault_type", Opt_fault_type),
+ fsparam_flag_no("lazytime", Opt_lazytime),
+ fsparam_flag_no("quota", Opt_quota),
+ fsparam_flag("usrquota", Opt_usrquota),
+ fsparam_flag("grpquota", Opt_grpquota),
+ fsparam_flag("prjquota", Opt_prjquota),
+ fsparam_string_empty("usrjquota", O
[f2fs-dev] [PATCH V3 2/7] f2fs: move the option parser into handle_mount_opt
From: Hongbo Li
In handle_mount_opt, we use fs_parameter to parse each option.
However we're still using the old API to get the options string.
Using fsparams parse_options allows us to remove many of the Opt_
enums, so remove them.
The checkpoint disable cap (or percent) involves rather complex
parsing; we retain the old match_table mechanism for this, which
handles it well.
There are some changes about parsing options:
1. For `active_logs`, `inline_xattr_size` and `fault_injection`,
we use s32 type according the internal structure to record the
option's value.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 1061 ++-
1 file changed, 406 insertions(+), 655 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ebea03bba054..20dee7c40d59 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "f2fs.h"
@@ -124,29 +125,20 @@ enum {
Opt_disable_roll_forward,
Opt_norecovery,
Opt_discard,
- Opt_nodiscard,
Opt_noheap,
Opt_heap,
Opt_user_xattr,
- Opt_nouser_xattr,
Opt_acl,
- Opt_noacl,
Opt_active_logs,
Opt_disable_ext_identify,
Opt_inline_xattr,
- Opt_noinline_xattr,
Opt_inline_xattr_size,
Opt_inline_data,
Opt_inline_dentry,
- Opt_noinline_dentry,
Opt_flush_merge,
- Opt_noflush_merge,
Opt_barrier,
- Opt_nobarrier,
Opt_fastboot,
Opt_extent_cache,
- Opt_noextent_cache,
- Opt_noinline_data,
Opt_data_flush,
Opt_reserve_root,
Opt_resgid,
@@ -155,21 +147,13 @@ enum {
Opt_fault_injection,
Opt_fault_type,
Opt_lazytime,
- Opt_nolazytime,
Opt_quota,
- Opt_noquota,
Opt_usrquota,
Opt_grpquota,
Opt_prjquota,
Opt_usrjquota,
Opt_grpjquota,
Opt_prjjquota,
- Opt_offusrjquota,
- Opt_offgrpjquota,
- Opt_offprjjquota,
- Opt_jqfmt_vfsold,
- Opt_jqfmt_vfsv0,
- Opt_jqfmt_vfsv1,
Opt_alloc,
Opt_fsync,
Opt_test_dummy_encryption,
@@ -179,17 +163,15 @@ enum {
Opt_checkpoint_disable_cap_perc,
Opt_checkpoint_enable,
Opt_checkpoint_merge,
- Opt_nocheckpoint_merge,
Opt_compress_algorithm,
Opt_compress_log_size,
- Opt_compress_extension,
Opt_nocompress_extension,
+ Opt_compress_extension,
Opt_compress_chksum,
Opt_compress_mode,
Opt_compress_cache,
Opt_atgc,
Opt_gc_merge,
- Opt_nogc_merge,
Opt_discard_unit,
Opt_memory_mode,
Opt_age_extent_cache,
@@ -319,83 +301,12 @@ static const struct fs_parameter_spec f2fs_param_specs[]
= {
{}
};
-static match_table_t f2fs_tokens = {
- {Opt_gc_background, "background_gc=%s"},
- {Opt_disable_roll_forward, "disable_roll_forward"},
- {Opt_norecovery, "norecovery"},
- {Opt_discard, "discard"},
- {Opt_nodiscard, "nodiscard"},
- {Opt_noheap, "no_heap"},
- {Opt_heap, "heap"},
- {Opt_user_xattr, "user_xattr"},
- {Opt_nouser_xattr, "nouser_xattr"},
- {Opt_acl, "acl"},
- {Opt_noacl, "noacl"},
- {Opt_active_logs, "active_logs=%u"},
- {Opt_disable_ext_identify, "disable_ext_identify"},
- {Opt_inline_xattr, "inline_xattr"},
- {Opt_noinline_xattr, "noinline_xattr"},
- {Opt_inline_xattr_size, "inline_xattr_size=%u"},
- {Opt_inline_data, "inline_data"},
- {Opt_inline_dentry, "inline_dentry"},
- {Opt_noinline_dentry, "noinline_dentry"},
- {Opt_flush_merge, "flush_merge"},
- {Opt_noflush_merge, "noflush_merge"},
- {Opt_barrier, "barrier"},
- {Opt_nobarrier, "nobarrier"},
- {Opt_fastboot, "fastboot"},
- {Opt_extent_cache, "extent_cache"},
- {Opt_noextent_cache, "noextent_cache"},
- {Opt_noinline_data, "noinline_data"},
- {Opt_data_flush, "data_flush"},
- {Opt_reserve_root, "reserve_root=%u"},
- {Opt_resgid, "resgid=%u"},
- {Opt_resuid, "resuid=%u"},
- {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"
Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion
On 4/1/25 3:33 PM, Eric Sandeen wrote:
> On 3/31/25 3:31 AM, Chao Yu wrote:
>> On 3/29/25 12:18, Eric Sandeen wrote:
>>> I was working on next steps for this, and I have a followup question.
>>>
>>> Today, several mount options are simply ignored if the on-disk format
>>> does not support them. For example:
>>>
>>> case Opt_compress_mode:
>>> if (!f2fs_sb_has_compression(sbi)) {
>>> f2fs_info(sbi, "Image doesn't support
>>> compression");
>>> break;
>>> }
>>> name = match_strdup(&args[0]);
>>> if (!name)
>>> return -ENOMEM;
>>> if (!strcmp(name, "fs")) {
>>> F2FS_OPTION(sbi).compress_mode =
>>> COMPR_MODE_FS;
>>> } else if (!strcmp(name, "user")) {
>>> F2FS_OPTION(sbi).compress_mode =
>>> COMPR_MODE_USER;
>>> } else {
>>> kfree(name);
>>> return -EINVAL;
>>> }
>>> kfree(name);
>>> break;
>>>
>>> so if f2fs_sb_has_compression() is not true, then the option is ignored
>>> without
>>> any validation.
>>>
>>> in other words, "mount -o compress_mode=nope ..." will succeed if the
>>> feature
>>> is disabled on the filesystem.
>>>
>>> If I move the f2fs_sb_has_compression() check to later for the new mount
>>> API,
>>> then "mount -o compress_mode=nope ..." will start failing for all images.
>>> Is
>>> this acceptable? It seems wise to reject invalid options rather than ignore
>>> them,
>>> even if they are incompatible with the format, but this would be a behavior
>>> change.
>>
>> I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
>> should stop passing invalid mount option to f2fs.
>
> Great, I will proceed with this. It will make the conversion simpler (but may
> make testing/validation more difficult, as behavior will change with invalid
> input).
FYI - I don't think I will be able to complete this conversion task myself -
f2fs is
by far the most difficult conversion I've encountered, and my time for these
sorts of
projects is sadly limited. I do have one more patch series that moves a lot of
the
on-disk feature checking out of option parsing, and perhaps I will send it as
an example
at least.
But I think it may be time to ask the f2fs experts to take over this effort,
because
I'm just not getting through it on my own.
(We are down to only a small handful of filesystems left - in fact, I think only
bfs, 9p, and f2fs, that don't have patches anywhere. So it would be really
great to
get some help on this.)
Thanks,
-Eric
> -Eric
>
>> Thanks,
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
Hi all - it would be nice to get some review or feedback on this; seems that these patches tend to go stale fairly quickly as f2fs evolves. :) Thanks, -Eric On 4/23/25 12:08 PM, Eric Sandeen wrote: > V3: > - Rebase onto git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git > dev branch > - Fix up some 0day robot warnings > > This is a forward-port of Hongbo's original f2fs mount API conversion, > posted last August at > https://lore.kernel.org/linux-f2fs-devel/20240814023912.3959299-1-lihongb...@huawei.com/ > > I had been trying to approach this with a little less complexity, > but in the end I realized that Hongbo's approach (which follows > the ext4 approach) was a good one, and I was not making any progrss > myself. 😉 > > In addition to the forward-port, I have also fixed a couple bugs I found > during testing, and some improvements / style choices as well. Hongbo and > I have discussed most of this off-list already, so I'm presenting the > net result here. > > This does pass my typical testing which does a large number of random > mounts/remounts with valid and invalid option sets, on f2fs filesystem > images with various features in the on-disk superblock. (I was not able > to test all of this completely, as some options or features require > hardware I dn't have.) > > Thanks, > -Eric > > (A recap of Hongbo's original cover letter is below, edited slightly for > this series:) > > Since many filesystems have done the new mount API conversion, > we introduce the new mount API conversion in f2fs. > > The series can be applied on top of the current mainline tree > and the work is based on the patches from Lukas Czerner (has > done this in ext4[1]). His patch give me a lot of ideas. > > Here is a high level description of the patchset: > > 1. Prepare the f2fs mount parameters required by the new mount > API and use it for parsing, while still using the old API to > get mount options string. Split the parameter parsing and > validation of the parse_options helper into two separate > helpers. > > f2fs: Add fs parameter specifications for mount options > f2fs: move the option parser into handle_mount_opt > > 2. Remove the use of sb/sbi structure of f2fs from all the > parsing code, because with the new mount API the parsing is > going to be done before we even get the super block. In this > part, we introduce f2fs_fs_context to hold the temporary > options when parsing. For the simple options check, it has > to be done during parsing by using f2fs_fs_context structure. > For the check which needs sb/sbi, we do this during super > block filling. > > f2fs: Allow sbi to be NULL in f2fs_printk > f2fs: Add f2fs_fs_context to record the mount options > f2fs: separate the options parsing and options checking > > 3. Switch the f2fs to use the new mount API for mount and > remount. > > f2fs: introduce fs_context_operation structure > f2fs: switch to the new mount api > > [1] https://lore.kernel.org/all/20211021114508.21407-1-lczer...@redhat.com/ > > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH AUTOSEL 6.14 156/642] f2fs: defer readonly check vs norecovery
This commit is in no way a bugfix and I don't see any reason to
backport it to stable kernels.
Thanks,
-Eric
On 5/5/25 5:06 PM, Sasha Levin wrote:
> From: Eric Sandeen
>
> [ Upstream commit 9cca49875997a1a7e92800a828a62bacb0f577b9 ]
>
> Defer the readonly-vs-norecovery check until after option parsing is done
> so that option parsing does not require an active superblock for the test.
> Add a helpful message, while we're at it.
>
> (I think could be moved back into parsing after we switch to the new mount
> API if desired, as the fs context will have RO state available.)
>
> Signed-off-by: Eric Sandeen
> Reviewed-by: Chao Yu
> Signed-off-by: Jaegeuk Kim
> Signed-off-by: Sasha Levin
> ---
> fs/f2fs/super.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index b8a0e925a4011..d3b04a589b525 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -728,10 +728,8 @@ static int parse_options(struct super_block *sb, char
> *options, bool is_remount)
> set_opt(sbi, DISABLE_ROLL_FORWARD);
> break;
> case Opt_norecovery:
> - /* this option mounts f2fs with ro */
> + /* requires ro mount, checked in f2fs_default_check */
> set_opt(sbi, NORECOVERY);
> - if (!f2fs_readonly(sb))
> - return -EINVAL;
> break;
> case Opt_discard:
> if (!f2fs_hw_support_discard(sbi)) {
> @@ -1418,6 +1416,12 @@ static int parse_options(struct super_block *sb, char
> *options, bool is_remount)
> f2fs_err(sbi, "Allow to mount readonly mode only");
> return -EROFS;
> }
> +
> + if (test_opt(sbi, NORECOVERY) && !f2fs_readonly(sbi->sb)) {
> + f2fs_err(sbi, "norecovery requires readonly mount");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion
On 4/12/25 12:17 PM, Eric Sandeen wrote: ... > FYI - I don't think I will be able to complete this conversion task myself - > f2fs is > by far the most difficult conversion I've encountered, and my time for these > sorts of > projects is sadly limited. I do have one more patch series that moves a lot > of the > on-disk feature checking out of option parsing, and perhaps I will send it as > an example > at least. > > But I think it may be time to ask the f2fs experts to take over this effort, > because > I'm just not getting through it on my own. > > (We are down to only a small handful of filesystems left - in fact, I think > only > bfs, 9p, and f2fs, that don't have patches anywhere. So it would be really > great to > get some help on this.) Sorry for replying to myself. Actually, I will take one more try at this - when I first looked at Hongo Li's patches to do the conversion, it seemed like there were a few problems, and I didn't review fully. I realize now that although it was a different approach than I had been taking, perhaps it was on the right track after all. I'll try to evaluate that patch series more carefully and see if it solved the problems I am struggling with now. If it does, I'd be happy to work with you, Hongbo, to get this completed. Your series will take a bit of forward-porting, as the codebase has moved forward quite a lot since then. But I'll go back to an older kernel and evaluate the net result of your changes, since I have some custom mount-option-testing framework here, and see how things look. Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/7] f2fs: move the option parser into handle_mount_opt
From: Hongbo Li
In handle_mount_opt, we use fs_parameter to parse each option.
However we're still using the old API to get the options string.
Using fsparams parse_options allows us to remove many of the Opt_
enums, so remove them.
The checkpoint disable cap (or percent) involves rather complex
parsing; we retain the old match_table mechanism for this, which
handles it well.
There are some changes about parsing options:
1. For `active_logs`, `inline_xattr_size` and `fault_injection`,
we use s32 type according the internal structure to record the
option's value.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 1063 ++-
1 file changed, 407 insertions(+), 656 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index c3623e052cde..8343dc2a515d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "f2fs.h"
@@ -122,29 +123,20 @@ enum {
Opt_disable_roll_forward,
Opt_norecovery,
Opt_discard,
- Opt_nodiscard,
Opt_noheap,
Opt_heap,
Opt_user_xattr,
- Opt_nouser_xattr,
Opt_acl,
- Opt_noacl,
Opt_active_logs,
Opt_disable_ext_identify,
Opt_inline_xattr,
- Opt_noinline_xattr,
Opt_inline_xattr_size,
Opt_inline_data,
Opt_inline_dentry,
- Opt_noinline_dentry,
Opt_flush_merge,
- Opt_noflush_merge,
Opt_barrier,
- Opt_nobarrier,
Opt_fastboot,
Opt_extent_cache,
- Opt_noextent_cache,
- Opt_noinline_data,
Opt_data_flush,
Opt_reserve_root,
Opt_resgid,
@@ -153,21 +145,13 @@ enum {
Opt_fault_injection,
Opt_fault_type,
Opt_lazytime,
- Opt_nolazytime,
Opt_quota,
- Opt_noquota,
Opt_usrquota,
Opt_grpquota,
Opt_prjquota,
Opt_usrjquota,
Opt_grpjquota,
Opt_prjjquota,
- Opt_offusrjquota,
- Opt_offgrpjquota,
- Opt_offprjjquota,
- Opt_jqfmt_vfsold,
- Opt_jqfmt_vfsv0,
- Opt_jqfmt_vfsv1,
Opt_alloc,
Opt_fsync,
Opt_test_dummy_encryption,
@@ -177,17 +161,15 @@ enum {
Opt_checkpoint_disable_cap_perc,
Opt_checkpoint_enable,
Opt_checkpoint_merge,
- Opt_nocheckpoint_merge,
Opt_compress_algorithm,
Opt_compress_log_size,
- Opt_compress_extension,
Opt_nocompress_extension,
+ Opt_compress_extension,
Opt_compress_chksum,
Opt_compress_mode,
Opt_compress_cache,
Opt_atgc,
Opt_gc_merge,
- Opt_nogc_merge,
Opt_discard_unit,
Opt_memory_mode,
Opt_age_extent_cache,
@@ -317,83 +299,12 @@ static const struct fs_parameter_spec f2fs_param_specs[]
= {
{}
};
-static match_table_t f2fs_tokens = {
- {Opt_gc_background, "background_gc=%s"},
- {Opt_disable_roll_forward, "disable_roll_forward"},
- {Opt_norecovery, "norecovery"},
- {Opt_discard, "discard"},
- {Opt_nodiscard, "nodiscard"},
- {Opt_noheap, "no_heap"},
- {Opt_heap, "heap"},
- {Opt_user_xattr, "user_xattr"},
- {Opt_nouser_xattr, "nouser_xattr"},
- {Opt_acl, "acl"},
- {Opt_noacl, "noacl"},
- {Opt_active_logs, "active_logs=%u"},
- {Opt_disable_ext_identify, "disable_ext_identify"},
- {Opt_inline_xattr, "inline_xattr"},
- {Opt_noinline_xattr, "noinline_xattr"},
- {Opt_inline_xattr_size, "inline_xattr_size=%u"},
- {Opt_inline_data, "inline_data"},
- {Opt_inline_dentry, "inline_dentry"},
- {Opt_noinline_dentry, "noinline_dentry"},
- {Opt_flush_merge, "flush_merge"},
- {Opt_noflush_merge, "noflush_merge"},
- {Opt_barrier, "barrier"},
- {Opt_nobarrier, "nobarrier"},
- {Opt_fastboot, "fastboot"},
- {Opt_extent_cache, "extent_cache"},
- {Opt_noextent_cache, "noextent_cache"},
- {Opt_noinline_data, "noinline_data"},
- {Opt_data_flush, "data_flush"},
- {Opt_reserve_root, "reserve_root=%u"},
- {Opt_resgid, "resgid=%u"},
- {Opt_resuid, "resuid=%u"},
- {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"
[f2fs-dev] [PATCH 4/7] f2fs: Add f2fs_fs_context to record the mount options
From: Hongbo Li
At the parsing phase of mouont in the new mount api, options
value will be recorded with the context, and then it will be
used in fill_super and other helpers.
Note that, this is a temporary status, we want remove the sb
and sbi usages in handle_mount_opt. So here the f2fs_fs_context
only records the mount options, it will be copied in sb/sbi in
later process. (At this point in the series, mount options are
temporarily not set during mount.)
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 419
1 file changed, 244 insertions(+), 175 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index aeb8e9a48bf6..11bf936b0d3c 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -308,8 +308,65 @@ static match_table_t f2fs_checkpoint_tokens = {
{Opt_err, NULL},
};
+#define F2FS_SPEC_background_gc(1 << 0)
+#define F2FS_SPEC_inline_xattr_size(1 << 1)
+#define F2FS_SPEC_active_logs (1 << 2)
+#define F2FS_SPEC_reserve_root (1 << 3)
+#define F2FS_SPEC_resgid (1 << 4)
+#define F2FS_SPEC_resuid (1 << 5)
+#define F2FS_SPEC_mode (1 << 6)
+#define F2FS_SPEC_fault_injection (1 << 7)
+#define F2FS_SPEC_fault_type (1 << 8)
+#define F2FS_SPEC_jqfmt(1 << 9)
+#define F2FS_SPEC_alloc_mode (1 << 10)
+#define F2FS_SPEC_fsync_mode (1 << 11)
+#define F2FS_SPEC_checkpoint_disable_cap (1 << 12)
+#define F2FS_SPEC_checkpoint_disable_cap_perc (1 << 13)
+#define F2FS_SPEC_compress_level (1 << 14)
+#define F2FS_SPEC_compress_algorithm (1 << 15)
+#define F2FS_SPEC_compress_log_size(1 << 16)
+#define F2FS_SPEC_compress_extension (1 << 17)
+#define F2FS_SPEC_nocompress_extension (1 << 18)
+#define F2FS_SPEC_compress_chksum (1 << 19)
+#define F2FS_SPEC_compress_mode(1 << 20)
+#define F2FS_SPEC_discard_unit (1 << 21)
+#define F2FS_SPEC_memory_mode (1 << 22)
+#define F2FS_SPEC_errors (1 << 23)
+
+struct f2fs_fs_context {
+ struct f2fs_mount_info info;
+ unsigned intopt_mask; /* Bits changed */
+ unsigned intspec_mask;
+ unsigned short qname_mask;
+ unsigned long sflags;
+ unsigned long sflags_mask;
+};
+
+#define F2FS_CTX_INFO(ctx) ((ctx)->info)
+
+static inline void ctx_set_opt(struct f2fs_fs_context *ctx,
+ unsigned int flag)
+{
+ ctx->info.opt |= flag;
+ ctx->opt_mask |= flag;
+}
+
+static inline void ctx_clear_opt(struct f2fs_fs_context *ctx,
+unsigned int flag)
+{
+ ctx->info.opt &= ~flag;
+ ctx->opt_mask |= flag;
+}
+
+static inline void ctx_set_flags(struct f2fs_fs_context *ctx,
+unsigned int flag)
+{
+ ctx->sflags |= flag;
+ ctx->sflags_mask |= flag;
+}
+
void f2fs_printk(struct f2fs_sb_info *sbi, bool limit_rate,
- const char *fmt, ...)
+ const char *fmt, ...)
{
struct va_format vaf;
va_list args;
@@ -427,57 +484,51 @@ static void init_once(void *foo)
#ifdef CONFIG_QUOTA
static const char * const quotatypes[] = INITQFNAMES;
#define QTYPE2NAME(t) (quotatypes[t])
-static int f2fs_set_qf_name(struct f2fs_sb_info *sbi, int qtype,
- struct fs_parameter *param)
+/*
+ * Note the name of the specified quota file.
+ */
+static int f2fs_note_qf_name(struct fs_context *fc, int qtype,
+struct fs_parameter *param)
{
- struct super_block *sb = sbi->sb;
+ struct f2fs_fs_context *ctx = fc->fs_private;
char *qname;
- int ret = -EINVAL;
- if (sb_any_quota_loaded(sb) && !F2FS_OPTION(sbi).s_qf_names[qtype]) {
- f2fs_err(sbi, "Cannot change journaled quota options when quota
turned on");
+ if (param->size < 1) {
+ f2fs_err(NULL, "Missing quota name");
return -EINVAL;
}
- if (f2fs_sb_has_quota_ino(sbi)) {
- f2fs_info(sbi, "QUOTA feature is enabled, so ignore qf_name");
+ if (strchr(param->string, '/')) {
+ f2fs_err(NULL, "quotafile must be on filesystem root");
+ return -EINVAL;
+ }
+ if (ctx->info.s_qf_names[qtype]) {
+ if (strcmp(ctx->info.s_qf_names[qtype
[f2fs-dev] [PATCH 5/7] f2fs: separate the options parsing and options checking
From: Hongbo Li
The new mount api separates option parsing and super block setup
into two distinct steps and so we need to separate the options
parsing out of the parse_options().
In order to achieve this, here we handle the mount options with
three steps:
- Firstly, we move sb/sbi out of handle_mount_opt.
As the former patch introduced f2fs_fs_context, so we record
the changed mount options in this context. In handle_mount_opt,
sb/sbi is null, so we should move all relative code out of
handle_mount_opt (thus, some check case which use sb/sbi should
move out).
- Secondly, we introduce the some check helpers to keep the option
consistent.
During filling superblock period, sb/sbi are ready. So we check
the f2fs_fs_context which holds the mount options base on sb/sbi.
- Thirdly, we apply the new mount options to sb/sbi.
After checking the f2fs_fs_context, all changed on mount options
are valid. So we can apply them to sb/sbi directly.
After do these, option parsing and super block setting have been
decoupled. Also it should have retained the original execution
flow.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 694 +++-
1 file changed, 510 insertions(+), 184 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 11bf936b0d3c..818db1e9549b 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -358,6 +358,12 @@ static inline void ctx_clear_opt(struct f2fs_fs_context
*ctx,
ctx->opt_mask |= flag;
}
+static inline bool ctx_test_opt(struct f2fs_fs_context *ctx,
+ unsigned int flag)
+{
+ return ctx->info.opt & flag;
+}
+
static inline void ctx_set_flags(struct f2fs_fs_context *ctx,
unsigned int flag)
{
@@ -531,51 +537,6 @@ static int f2fs_unnote_qf_name(struct fs_context *fc, int
qtype)
ctx->qname_mask |= 1 << qtype;
return 0;
}
-
-static int f2fs_check_quota_options(struct f2fs_sb_info *sbi)
-{
- /*
-* We do the test below only for project quotas. 'usrquota' and
-* 'grpquota' mount options are allowed even without quota feature
-* to support legacy quotas in quota files.
-*/
- if (test_opt(sbi, PRJQUOTA) && !f2fs_sb_has_project_quota(sbi)) {
- f2fs_err(sbi, "Project quota feature not enabled. Cannot enable
project quota enforcement.");
- return -1;
- }
- if (F2FS_OPTION(sbi).s_qf_names[USRQUOTA] ||
- F2FS_OPTION(sbi).s_qf_names[GRPQUOTA] ||
- F2FS_OPTION(sbi).s_qf_names[PRJQUOTA]) {
- if (test_opt(sbi, USRQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[USRQUOTA])
- clear_opt(sbi, USRQUOTA);
-
- if (test_opt(sbi, GRPQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[GRPQUOTA])
- clear_opt(sbi, GRPQUOTA);
-
- if (test_opt(sbi, PRJQUOTA) &&
- F2FS_OPTION(sbi).s_qf_names[PRJQUOTA])
- clear_opt(sbi, PRJQUOTA);
-
- if (test_opt(sbi, GRPQUOTA) || test_opt(sbi, USRQUOTA) ||
- test_opt(sbi, PRJQUOTA)) {
- f2fs_err(sbi, "old and new quota format mixing");
- return -1;
- }
-
- if (!F2FS_OPTION(sbi).s_jquota_fmt) {
- f2fs_err(sbi, "journaled quota format not specified");
- return -1;
- }
- }
-
- if (f2fs_sb_has_quota_ino(sbi) && F2FS_OPTION(sbi).s_jquota_fmt) {
- f2fs_info(sbi, "QUOTA feature is enabled, so ignore
jquota_fmt");
- F2FS_OPTION(sbi).s_jquota_fmt = 0;
- }
- return 0;
-}
#endif
static int f2fs_parse_test_dummy_encryption(const struct fs_parameter *param,
@@ -634,28 +595,28 @@ static bool is_compress_extension_exist(struct
f2fs_mount_info *info,
* extension will be treated as special cases and will not be compressed.
* 3. Don't allow the non-compress extension specifies all files.
*/
-static int f2fs_test_compress_extension(struct f2fs_sb_info *sbi)
+static int f2fs_test_compress_extension(unsigned char
(*noext)[F2FS_EXTENSION_LEN],
+ int noext_cnt,
+ unsigned char
(*ext)[F2FS_EXTENSION_LEN],
+ int ext_cnt)
{
- unsigned char (*ext)[F2FS_EXTENSION_LEN];
- unsigned char (*noext)[F2FS_EXTENSION_LEN];
- int ext_cnt, noext_cnt, index = 0, no_index = 0;
-
- ext = F2FS_OPTION(sbi).extensions;
- ext_cnt = F2FS_OPTION(sbi).compress_ext_cn
[f2fs-dev] [PATCH 6/7] f2fs: introduce fs_context_operation structure
From: Hongbo Li
The handle_mount_opt() helper is used to parse mount parameters,
and so we can rename this function to f2fs_parse_param() and set
it as .param_param in fs_context_operations.
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 818db1e9549b..21042a02459f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -705,7 +705,7 @@ static int f2fs_set_zstd_level(struct f2fs_fs_context *ctx,
const char *str)
#endif
#endif
-static int handle_mount_opt(struct fs_context *fc, struct fs_parameter *param)
+static int f2fs_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct f2fs_fs_context *ctx = fc->fs_private;
#ifdef CONFIG_F2FS_FS_COMPRESSION
@@ -1171,7 +1171,7 @@ static int parse_options(struct fs_context *fc, char
*options)
param.key = key;
param.size = v_len;
- ret = handle_mount_opt(fc, ¶m);
+ ret = f2fs_parse_param(fc, ¶m);
kfree(param.string);
if (ret < 0)
return ret;
@@ -5273,6 +5273,10 @@ static struct dentry *f2fs_mount(struct file_system_type
*fs_type, int flags,
return mount_bdev(fs_type, flags, dev_name, data, f2fs_fill_super);
}
+static const struct fs_context_operations f2fs_context_ops = {
+ .parse_param= f2fs_parse_param,
+};
+
static void kill_f2fs_super(struct super_block *sb)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
--
2.47.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 7/7] f2fs: switch to the new mount api
From: Hongbo Li
The new mount api will execute .parse_param, .init_fs_context, .get_tree
and will call .remount if remount happened. So we add the necessary
functions for the fs_context_operations. If .init_fs_context is added,
the old .mount should remove.
See Documentation/filesystems/mount_api.rst for more information.
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 156 +++-
1 file changed, 62 insertions(+), 94 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 21042a02459f..28a7aa01d009 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1139,47 +1139,6 @@ static int f2fs_parse_param(struct fs_context *fc,
struct fs_parameter *param)
return 0;
}
-static int parse_options(struct fs_context *fc, char *options)
-{
- struct fs_parameter param;
- char *key;
- int ret;
-
- if (!options)
- return 0;
-
- while ((key = strsep(&options, ",")) != NULL) {
- if (*key) {
- size_t v_len = 0;
- char *value = strchr(key, '=');
-
- param.type = fs_value_is_flag;
- param.string = NULL;
-
- if (value) {
- if (value == key)
- continue;
-
- *value++ = 0;
- v_len = strlen(value);
- param.string = kmemdup_nul(value, v_len,
GFP_KERNEL);
- if (!param.string)
- return -ENOMEM;
- param.type = fs_value_is_string;
- }
-
- param.key = key;
- param.size = v_len;
-
- ret = f2fs_parse_param(fc, ¶m);
- kfree(param.string);
- if (ret < 0)
- return ret;
- }
- }
- return 0;
-}
-
/*
* Check quota settings consistency.
*/
@@ -2579,13 +2538,12 @@ static void f2fs_enable_checkpoint(struct f2fs_sb_info
*sbi)
f2fs_flush_ckpt_thread(sbi);
}
-static int f2fs_remount(struct super_block *sb, int *flags, char *data)
+static int __f2fs_remount(struct fs_context *fc, struct super_block *sb)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
struct f2fs_mount_info org_mount_opt;
- struct f2fs_fs_context ctx;
- struct fs_context fc;
unsigned long old_sb_flags;
+ unsigned int flags = fc->sb_flags;
int err;
bool need_restart_gc = false, need_stop_gc = false;
bool need_restart_flush = false, need_stop_flush = false;
@@ -2631,7 +2589,7 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
#endif
/* recover superblocks we couldn't write due to previous RO mount */
- if (!(*flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
+ if (!(flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
err = f2fs_commit_super(sbi, false);
f2fs_info(sbi, "Try to recover all the superblocks, ret: %d",
err);
@@ -2641,21 +2599,11 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
default_options(sbi, true);
- memset(&fc, 0, sizeof(fc));
- memset(&ctx, 0, sizeof(ctx));
- fc.fs_private = &ctx;
- fc.purpose = FS_CONTEXT_FOR_RECONFIGURE;
-
- /* parse mount options */
- err = parse_options(&fc, data);
- if (err)
- goto restore_opts;
-
- err = f2fs_check_opt_consistency(&fc, sb);
+ err = f2fs_check_opt_consistency(fc, sb);
if (err < 0)
goto restore_opts;
- f2fs_apply_options(&fc, sb);
+ f2fs_apply_options(fc, sb);
#ifdef CONFIG_BLK_DEV_ZONED
if (f2fs_sb_has_blkzoned(sbi) &&
@@ -2674,20 +2622,20 @@ static int f2fs_remount(struct super_block *sb, int
*flags, char *data)
* Previous and new state of filesystem is RO,
* so skip checking GC and FLUSH_MERGE conditions.
*/
- if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
+ if (f2fs_readonly(sb) && (flags & SB_RDONLY))
goto skip;
- if (f2fs_dev_is_readonly(sbi) && !(*flags & SB_RDONLY)) {
+ if (f2fs_dev_is_readonly(sbi) && !(flags & SB_RDONLY)) {
err = -EROFS;
goto restore_opts;
}
#ifdef CONFIG_QUOTA
- if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
+ if (!f2fs_readonly(sb) && (flags & SB_RDONLY)) {
err = dquot_suspend(sb, -1);
if (
[f2fs-dev] [PATCH 0/7 V2] f2fs: new mount API conversion
This is a forward-port of Hongbo's original f2fs mount API conversion, posted last August at https://lore.kernel.org/linux-f2fs-devel/20240814023912.3959299-1-lihongb...@huawei.com/ I had been trying to approach this with a little less complexity, but in the end I realized that Hongbo's approach (which follows the ext4 approach) was a good one, and I was not making any progrss myself. ;) In addition to the forward-port, I have also fixed a couple bugs I found during testing, and some improvements / style choices as well. Hongbo and I have discussed most of this off-list already, so I'm presenting the net result here. This does pass my typical testing which does a large number of random mounts/remounts with valid and invalid option sets, on f2fs filesystem images with various features in the on-disk superblock. (I was not able to test all of this completely, as some options or features require hardware I dn't have.) Thanks, -Eric (A recap of Hongbo's original cover letter is below, edited slightly for this series:) Since many filesystems have done the new mount API conversion, we introduce the new mount API conversion in f2fs. The series can be applied on top of the current mainline tree and the work is based on the patches from Lukas Czerner (has done this in ext4[1]). His patch give me a lot of ideas. Here is a high level description of the patchset: 1. Prepare the f2fs mount parameters required by the new mount API and use it for parsing, while still using the old API to get mount options string. Split the parameter parsing and validation of the parse_options helper into two separate helpers. f2fs: Add fs parameter specifications for mount options f2fs: move the option parser into handle_mount_opt 2. Remove the use of sb/sbi structure of f2fs from all the parsing code, because with the new mount API the parsing is going to be done before we even get the super block. In this part, we introduce f2fs_fs_context to hold the temporary options when parsing. For the simple options check, it has to be done during parsing by using f2fs_fs_context structure. For the check which needs sb/sbi, we do this during super block filling. f2fs: Allow sbi to be NULL in f2fs_printk f2fs: Add f2fs_fs_context to record the mount options f2fs: separate the options parsing and options checking 3. Switch the f2fs to use the new mount API for mount and remount. f2fs: introduce fs_context_operation structure f2fs: switch to the new mount api [1] https://lore.kernel.org/all/20211021114508.21407-1-lczer...@redhat.com/ ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 3/7] f2fs: Allow sbi to be NULL in f2fs_printk
From: Hongbo Li
At the parsing phase of the new mount api, sbi will not be
available. So here allows sbi to be NULL in f2fs log helpers
and use that in handle_mount_opt().
Signed-off-by: Hongbo Li
[sandeen: forward port]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 90 +++--
1 file changed, 49 insertions(+), 41 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8343dc2a515d..aeb8e9a48bf6 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -321,11 +321,19 @@ void f2fs_printk(struct f2fs_sb_info *sbi, bool
limit_rate,
vaf.fmt = printk_skip_level(fmt);
vaf.va = &args;
if (limit_rate)
- printk_ratelimited("%c%cF2FS-fs (%s): %pV\n",
- KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ if (sbi)
+ printk_ratelimited("%c%cF2FS-fs (%s): %pV\n",
+ KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ else
+ printk_ratelimited("%c%cF2FS-fs: %pV\n",
+ KERN_SOH_ASCII, level, &vaf);
else
- printk("%c%cF2FS-fs (%s): %pV\n",
- KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ if (sbi)
+ printk("%c%cF2FS-fs (%s): %pV\n",
+ KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);
+ else
+ printk("%c%cF2FS-fs: %pV\n",
+ KERN_SOH_ASCII, level, &vaf);
va_end(args);
}
@@ -735,13 +743,13 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_discard:
if (result.negated) {
if (f2fs_hw_should_discard(sbi)) {
- f2fs_warn(sbi, "discard is required for zoned
block devices");
+ f2fs_warn(NULL, "discard is required for zoned
block devices");
return -EINVAL;
}
clear_opt(sbi, DISCARD);
} else {
if (!f2fs_hw_support_discard(sbi)) {
- f2fs_warn(sbi, "device does not support
discard");
+ f2fs_warn(NULL, "device does not support
discard");
break;
}
set_opt(sbi, DISCARD);
@@ -749,7 +757,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
case Opt_noheap:
case Opt_heap:
- f2fs_warn(sbi, "heap/no_heap options were deprecated");
+ f2fs_warn(NULL, "heap/no_heap options were deprecated");
break;
#ifdef CONFIG_F2FS_FS_XATTR
case Opt_user_xattr:
@@ -772,7 +780,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_user_xattr:
case Opt_inline_xattr:
case Opt_inline_xattr_size:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -784,7 +792,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
#else
case Opt_acl:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
case Opt_active_logs:
@@ -838,7 +846,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
break;
case Opt_reserve_root:
if (test_opt(sbi, RESERVE_ROOT)) {
- f2fs_info(sbi, "Preserve previous reserve_root=%u",
+ f2fs_info(NULL, "Preserve previous reserve_root=%u",
F2FS_OPTION(sbi).root_reserved_blocks);
} else {
F2FS_OPTION(sbi).root_reserved_blocks = result.int_32;
@@ -870,7 +878,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
#else
case Opt_fault_injection:
case Opt_fault_type:
- f2fs_info(sbi, "%s options not supported", param->key);
+ f2fs_info(NULL, "%s options not supported", param->key);
break;
#endif
case Opt_lazytime:
@@ -933,7 +941,7 @@ static int handle_mount_opt(struct fs_context *fc, struct
fs_parameter *param)
case Opt_usrjquota:
case Opt_grpjquota:
[f2fs-dev] [PATCH 1/7] f2fs: Add fs parameter specifications for mount options
From: Hongbo Li
Use an array of `fs_parameter_spec` called f2fs_param_specs to
hold the mount option specifications for the new mount api.
Add constant_table structures for several options to facilitate
parsing.
Signed-off-by: Hongbo Li
[sandeen: forward port, minor fixes and updates, more fsparam_enum]
Signed-off-by: Eric Sandeen
---
fs/f2fs/super.c | 122
1 file changed, 122 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f087b2b71c89..c3623e052cde 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "f2fs.h"
#include "node.h"
@@ -192,9 +193,130 @@ enum {
Opt_age_extent_cache,
Opt_errors,
Opt_nat_bits,
+ Opt_jqfmt,
+ Opt_checkpoint,
Opt_err,
};
+static const struct constant_table f2fs_param_background_gc[] = {
+ {"on", BGGC_MODE_ON},
+ {"off", BGGC_MODE_OFF},
+ {"sync",BGGC_MODE_SYNC},
+ {}
+};
+
+static const struct constant_table f2fs_param_mode[] = {
+ {"adaptive",FS_MODE_ADAPTIVE},
+ {"lfs", FS_MODE_LFS},
+ {"fragment:segment",FS_MODE_FRAGMENT_SEG},
+ {"fragment:block", FS_MODE_FRAGMENT_BLK},
+ {}
+};
+
+static const struct constant_table f2fs_param_jqfmt[] = {
+ {"vfsold", QFMT_VFS_OLD},
+ {"vfsv0", QFMT_VFS_V0},
+ {"vfsv1", QFMT_VFS_V1},
+ {}
+};
+
+static const struct constant_table f2fs_param_alloc_mode[] = {
+ {"default", ALLOC_MODE_DEFAULT},
+ {"reuse", ALLOC_MODE_REUSE},
+ {}
+};
+static const struct constant_table f2fs_param_fsync_mode[] = {
+ {"posix", FSYNC_MODE_POSIX},
+ {"strict", FSYNC_MODE_STRICT},
+ {"nobarrier", FSYNC_MODE_NOBARRIER},
+ {}
+};
+
+static const struct constant_table f2fs_param_compress_mode[] = {
+ {"fs", COMPR_MODE_FS},
+ {"user",COMPR_MODE_USER},
+ {}
+};
+
+static const struct constant_table f2fs_param_discard_unit[] = {
+ {"block", DISCARD_UNIT_BLOCK},
+ {"segment", DISCARD_UNIT_SEGMENT},
+ {"section", DISCARD_UNIT_SECTION},
+ {}
+};
+
+static const struct constant_table f2fs_param_memory_mode[] = {
+ {"normal", MEMORY_MODE_NORMAL},
+ {"low", MEMORY_MODE_LOW},
+ {}
+};
+
+static const struct constant_table f2fs_param_errors[] = {
+ {"remount-ro", MOUNT_ERRORS_READONLY},
+ {"continue",MOUNT_ERRORS_CONTINUE},
+ {"panic", MOUNT_ERRORS_PANIC},
+ {}
+};
+
+static const struct fs_parameter_spec f2fs_param_specs[] = {
+ fsparam_enum("background_gc", Opt_gc_background,
f2fs_param_background_gc),
+ fsparam_flag("disable_roll_forward", Opt_disable_roll_forward),
+ fsparam_flag("norecovery", Opt_norecovery),
+ fsparam_flag_no("discard", Opt_discard),
+ fsparam_flag("no_heap", Opt_noheap),
+ fsparam_flag("heap", Opt_heap),
+ fsparam_flag_no("user_xattr", Opt_user_xattr),
+ fsparam_flag_no("acl", Opt_acl),
+ fsparam_s32("active_logs", Opt_active_logs),
+ fsparam_flag("disable_ext_identify", Opt_disable_ext_identify),
+ fsparam_flag_no("inline_xattr", Opt_inline_xattr),
+ fsparam_s32("inline_xattr_size", Opt_inline_xattr_size),
+ fsparam_flag_no("inline_data", Opt_inline_data),
+ fsparam_flag_no("inline_dentry", Opt_inline_dentry),
+ fsparam_flag_no("flush_merge", Opt_flush_merge),
+ fsparam_flag_no("barrier", Opt_barrier),
+ fsparam_flag("fastboot", Opt_fastboot),
+ fsparam_flag_no("extent_cache", Opt_extent_cache),
+ fsparam_flag("data_flush", Opt_data_flush),
+ fsparam_u32("reserve_root", Opt_reserve_root),
+ fsparam_gid("resgid", Opt_resgid),
+ fsparam_uid("resuid", Opt_resuid),
+ fsparam_enum("mode", Opt_mode, f2fs_param_mode),
+ fsparam_s32("fault_injection", Opt_fault_injection),
+ fsparam_u32("fault_type", Opt_fault_type),
+ fsparam_flag_no("lazytime", Opt_lazytime),
+ fsparam_flag_no("quota", Opt_quota),
+ fsparam_flag("usrquota", Opt_usrquota),
+ fsparam_flag("grpquota", Opt_grpquota),
+ fsparam_flag("prjquota", Opt_prjquota),
+ fsparam_string_empty("usrjquota", O
Re: [f2fs-dev] [PATCH 2/7] f2fs: move the option parser into handle_mount_opt
On 5/7/25 6:26 AM, Chao Yu wrote: > On 4/20/25 23:25, Eric Sandeen wrote: >> From: Hongbo Li >> >> In handle_mount_opt, we use fs_parameter to parse each option. >> However we're still using the old API to get the options string. >> Using fsparams parse_options allows us to remove many of the Opt_ >> enums, so remove them. >> >> The checkpoint disable cap (or percent) involves rather complex >> parsing; we retain the old match_table mechanism for this, which >> handles it well. >> >> There are some changes about parsing options: >> 1. For `active_logs`, `inline_xattr_size` and `fault_injection`, >> we use s32 type according the internal structure to record the >> option's value. > > We'd better to use u32 type for these options, as they should never > be negative. > > Can you please update based on below patch? > > https://lore.kernel.org/linux-f2fs-devel/20250507112425.939246-1-c...@kernel.org Hi Chao - I agree that that patch makes sense, but maybe there is a timing issue now? At the moment, there is a mix of signed and unsigned handling for these options. I agree that the conversion series probably should have left the parsing type as unsigned, but it was a mix internally, so it was difficult to know for sure. For your patch above, if it is to stand alone or be merged first, it should probably also change the current parsing to match_uint. (this would also make it backportable to -stable kernels, if you want to). Otherwise, I would suggest that if it is merged after the mount API series, then your patch to clean up internal types could fix the (new mount API) parsing from %s to %u at the same time? Happy to do it either way but your patch should probably be internally consistent, changing the parsing types at the same time. (I suppose we could incorporate your patch into the mount API series too, though it'd be a little strange to have a minor bugfix like this buried in the series.) Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/7/25 9:46 AM, Jaegeuk Kim wrote:
> I meant:
>
> # mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb
> # mount /dev/vdb mnt
>
> It's supposed to be successful, since extent_cache is enabled by default.
I'm sorry, clearly I was too sleepy last night. This fixes it for me.
We have to test the mask to see if the option was explisitly set (either
extent_cache or noextent_cache) at mount time.
If it was not specified at all, it will be set by the default f'n and
remain in the sbi, and it will pass this consistency check.
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d89b9ede221e..e178796ce9a7 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1412,7 +1414,8 @@ static int f2fs_check_opt_consistency(struct fs_context
*fc,
}
if (f2fs_sb_has_device_alias(sbi) &&
- !ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
+ (ctx->opt_mask & F2FS_MOUNT_READ_EXTENT_CACHE) &&
+!ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
f2fs_err(sbi, "device aliasing requires extent cache");
return -EINVAL;
}
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 5/7] f2fs: separate the options parsing and options checking
On 5/8/25 3:13 AM, Chao Yu wrote:
> On 4/24/25 01:08, Eric Sandeen wrote:
>> From: Hongbo Li
...
>> +if (ctx->qname_mask) {
>> +for (i = 0; i < MAXQUOTAS; i++) {
>> +if (!(ctx->qname_mask & (1 << i)))
>> +continue;
>> +
>> +old_qname = F2FS_OPTION(sbi).s_qf_names[i];
>> +new_qname = F2FS_CTX_INFO(ctx).s_qf_names[i];
>> +if (quota_turnon &&
>> +!!old_qname != !!new_qname)
>> +goto err_jquota_change;
>> +
>> +if (old_qname) {
>> +if (strcmp(old_qname, new_qname) == 0) {
>> +ctx->qname_mask &= ~(1 << i);
>
> Needs to free and nully F2FS_CTX_INFO(ctx).s_qf_names[i]?
>
I will have to look into this. If s_qf_names are used/applied, they get
transferred to the sbi in f2fs_apply_quota_options and are freed in the
normal course of the fiesystem lifetime, i.e at unmount in f2fs_put_super.
That's the normal non-error lifecycle of the strings.
If they do not get transferred to the sbi in f2fs_apply_quota_options, they
remain on the ctx, and should get freed in f2fs_fc_free:
for (i = 0; i < MAXQUOTAS; i++)
kfree(F2FS_CTX_INFO(ctx).s_qf_names[i]);
It is possible to free them before f2fs_fc_free of course and that might
be an inconsistency in this function, because we do that in the other case
in the check_consistency function:
if (quota_feature) {
f2fs_info(sbi, "QUOTA feature is enabled, so
ignore qf_name");
ctx->qname_mask &= ~(1 << i);
kfree(F2FS_CTX_INFO(ctx).s_qf_names[i]);
F2FS_CTX_INFO(ctx).s_qf_names[i] = NULL;
}
I'll have to look at it a bit more. But this is modeled on ext4's
ext4_check_quota_consistency(), and it does not do any freeing in that
function; it leaves freeing in error cases to when the fc / ctx gets freed.
But tl;dr: I think we can remove the kfree and "= NULL" in this function,
and defer the freeing in the error case.
>> +
>> +static inline void clear_compression_spec(struct f2fs_fs_context *ctx)
>> +{
>> +ctx->spec_mask &= ~(F2FS_SPEC_compress_algorithm
>> +| F2FS_SPEC_compress_log_size
>> +| F2FS_SPEC_compress_extension
>> +| F2FS_SPEC_nocompress_extension
>> +| F2FS_SPEC_compress_chksum
>> +| F2FS_SPEC_compress_mode);
>
> How about add a macro to include all compression macros, and use it to clean
> up above codes?
That's a good idea and probably easy enough to do without rebase pain.
>> +
>> +if (f2fs_test_compress_extension(F2FS_CTX_INFO(ctx).noextensions,
>> +F2FS_CTX_INFO(ctx).nocompress_ext_cnt,
>> +F2FS_CTX_INFO(ctx).extensions,
>> +F2FS_CTX_INFO(ctx).compress_ext_cnt)) {
>> +f2fs_err(sbi, "invalid compress or nocompress extension");
>
> Can you please describe what is detailed confliction in the log? e.g. new
> noext conflicts w/ new ext...
Hmm, let me think about this. I had not noticed it was calling
f2fs_test_compress_extension 3 times, I wonder if there is a better option.
I need to understand this approach better. Maybe Hongbo has thoughts.
>> +return -EINVAL;
>> +}
>> +if (f2fs_test_compress_extension(F2FS_CTX_INFO(ctx).noextensions,
>> +F2FS_CTX_INFO(ctx).nocompress_ext_cnt,
>> +F2FS_OPTION(sbi).extensions,
>> +F2FS_OPTION(sbi).compress_ext_cnt)) {
>> +f2fs_err(sbi, "invalid compress or nocompress extension");
>
> Ditto,
>
>> +return -EINVAL;
>> +}
>> +if (f2fs_test_compress_extension(F2FS_OPTION(sbi).noextensions,
>> +F2FS_OPTION(sbi).nocompress_ext_cnt,
>> +F2FS_CTX_INFO(ctx).extensions,
>> +F2FS_CTX_INFO(ctx).compress_ext_cnt)) {
>> +f2fs_err(sbi, "invalid compress or nocompress extension");
>
> Ditto,
thanks,
-Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 7/7] f2fs: switch to the new mount api
On 5/8/25 4:19 AM, Chao Yu wrote:
>> @@ -2645,21 +2603,11 @@ static int f2fs_remount(struct super_block *sb, int
>> *flags, char *data)
>>
>> default_options(sbi, true);
>>
>> -memset(&fc, 0, sizeof(fc));
>> -memset(&ctx, 0, sizeof(ctx));
>> -fc.fs_private = &ctx;
>> -fc.purpose = FS_CONTEXT_FOR_RECONFIGURE;
>> -
>> -/* parse mount options */
>> -err = parse_options(&fc, data);
>> -if (err)
>> -goto restore_opts;
> There is a retry flow during f2fs_fill_super(), I intenionally inject a
> fault into f2fs_fill_super() to trigger the retry flow, it turns out that
> mount option may be missed w/ below testcase:
I never did understand that retry logic (introduced in ed2e621a95d long
ago). What errors does it expect to be able to retry, with success?
Anyway ...
Can you show me (as a patch) exactly what you did to trigger the retry,
just so we are looking at the same thing?
> - mkfs.f2fs -f -O encrypt /dev/vdb
> - mount -o test_dummy_encryption /dev/vdb /mnt/f2fs/
> : return success
> - dmesg -c
>
> [ 83.619982] f2fs_fill_super, retry_cnt:1
> [ 83.620914] F2FS-fs (vdb): Test dummy encryption mode enabled
> [ 83.668380] f2fs_fill_super, retry_cnt:0
> [ 83.671601] F2FS-fs (vdb): Mounted with checkpoint version = 7a8dfca5
>
> - mount|grep f2fs
> /dev/vdb on /mnt/f2fs type f2fs
> (rw,relatime,lazytime,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)
>
> The reason may be it has cleared F2FS_CTX_INFO(ctx).dummy_enc_policy in
> f2fs_apply_test_dummy_encryption().
>
> static void f2fs_apply_test_dummy_encryption(struct fs_context *fc,
>struct super_block *sb)
> {
> struct f2fs_fs_context *ctx = fc->fs_private;
> struct f2fs_sb_info *sbi = F2FS_SB(sb);
>
> if (!fscrypt_is_dummy_policy_set(&F2FS_CTX_INFO(ctx).dummy_enc_policy)
> ||
> /* if already set, it was already verified to be the same */
> fscrypt_is_dummy_policy_set(&F2FS_OPTION(sbi).dummy_enc_policy))
> return;
> F2FS_OPTION(sbi).dummy_enc_policy = F2FS_CTX_INFO(ctx).dummy_enc_policy;
> memset(&F2FS_CTX_INFO(ctx).dummy_enc_policy, 0,
> sizeof(F2FS_CTX_INFO(ctx).dummy_enc_policy));
> f2fs_warn(sbi, "Test dummy encryption mode enabled");
> }
>
> Can we save old mount_info from sbi or ctx from fc, and try to recover it
> before we retry mount flow?
I'll have to take more time to understand this concern. But thanks for pointing
it out.
-Eric
> Thanks,
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/7/25 2:48 PM, Jaegeuk Kim wrote:
> On 05/07, Eric Sandeen wrote:
>> On 5/7/25 9:46 AM, Jaegeuk Kim wrote:
>>
>>> I meant:
>>>
>>> # mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb
>>> # mount /dev/vdb mnt
>>>
>>> It's supposed to be successful, since extent_cache is enabled by default.
>>
>> I'm sorry, clearly I was too sleepy last night. This fixes it for me.
>>
>> We have to test the mask to see if the option was explisitly set (either
>> extent_cache or noextent_cache) at mount time.
>>
>> If it was not specified at all, it will be set by the default f'n and
>> remain in the sbi, and it will pass this consistency check.
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index d89b9ede221e..e178796ce9a7 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1412,7 +1414,8 @@ static int f2fs_check_opt_consistency(struct
>> fs_context *fc,
>> }
>>
>> if (f2fs_sb_has_device_alias(sbi) &&
>> -!ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
>> +(ctx->opt_mask & F2FS_MOUNT_READ_EXTENT_CACHE) &&
>> + !ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
>> f2fs_err(sbi, "device aliasing requires extent cache");
>> return -EINVAL;
>> }
>
> I think that will cover the user-given options only, but we'd better check the
> final options as well. Can we apply like this?
I'm sorry, I'm not sure I understand what situation this additional
changes will cover...
It looks like this adds the f2fs_sanity_check_options() to the remount
path to explicitly (re-)check a few things.
But as far as I can tell, at least for the extent cache, remount is handled
properly already (with the hunk above):
# mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb
# mount /dev/vdb mnt
# mount -o remount,noextent_cache mnt
mount: /root/mnt: mount point not mounted or bad option.
dmesg(1) may have more information after failed mount system call.
# dmesg | tail -n 1
[60012.364941] F2FS-fs (vdb): device aliasing requires extent cache
#
I haven't tested with i.e. blkzoned devices though, is there a testcase
that fails for you?
Thanks,
-Eric
> ---
> fs/f2fs/super.c | 50 -
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d89b9ede221e..270a9bf9773d 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1412,6 +1412,7 @@ static int f2fs_check_opt_consistency(struct fs_context
> *fc,
> }
>
> if (f2fs_sb_has_device_alias(sbi) &&
> + (ctx->opt_mask & F2FS_MOUNT_READ_EXTENT_CACHE) &&
> !ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
> f2fs_err(sbi, "device aliasing requires extent cache");
> return -EINVAL;
> @@ -1657,6 +1658,29 @@ static void f2fs_apply_options(struct fs_context *fc,
> struct super_block *sb)
> f2fs_apply_quota_options(fc, sb);
> }
>
> +static int f2fs_sanity_check_options(struct f2fs_sb_info *sbi)
> +{
> + if (f2fs_sb_has_device_alias(sbi) &&
> + !test_opt(sbi, READ_EXTENT_CACHE)) {
> + f2fs_err(sbi, "device aliasing requires extent cache");
> + return -EINVAL;
> + }
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (f2fs_sb_has_blkzoned(sbi) &&
> + sbi->max_open_zones < F2FS_OPTION(sbi).active_logs) {
> + f2fs_err(sbi,
> + "zoned: max open zones %u is too small, need at least
> %u open zones",
> + sbi->max_open_zones,
> F2FS_OPTION(sbi).active_logs);
> + return -EINVAL;
> + }
> +#endif
> + if (f2fs_lfs_mode(sbi) && !IS_F2FS_IPU_DISABLE(sbi)) {
> + f2fs_warn(sbi, "LFS is not compatible with IPU");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> static struct inode *f2fs_alloc_inode(struct super_block *sb)
> {
> struct f2fs_inode_info *fi;
> @@ -2616,21 +2640,15 @@ static int __f2fs_remount(struct fs_context *fc,
> struct super_block *sb)
> default_options(sbi, true);
>
> err = f2fs_check_opt_consistency(fc, sb);
> - if (err < 0)
> + if (err)
> goto restore_opts;
>
> f2fs_apply_options(fc, sb);
>
> -#ifdef CONFIG_BLK_DEV_ZONED
> - if (f2fs_sb_has_blkzoned(sbi) &&
> - sbi->max_open_zones &
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/7/25 3:28 PM, Jaegeuk Kim wrote: >> But as far as I can tell, at least for the extent cache, remount is handled >> properly already (with the hunk above): >> >> # mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb >> # mount /dev/vdb mnt >> # mount -o remount,noextent_cache mnt >> mount: /root/mnt: mount point not mounted or bad option. >>dmesg(1) may have more information after failed mount system call. >> # dmesg | tail -n 1 >> [60012.364941] F2FS-fs (vdb): device aliasing requires extent cache >> # >> >> I haven't tested with i.e. blkzoned devices though, is there a testcase >> that fails for you? > I'm worrying about any missing case to check options enabled by > default_options. > For example, in the case of device_aliasing, we rely on enabling extent_cache > by default_options, which was not caught by f2fs_check_opt_consistency. > > I was thinking that we'd need a post sanity check. I see. If you want a "belt and suspenders" approach and it works for you, no argument from me :) -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 5/7] f2fs: separate the options parsing and options checking
On 5/8/25 10:52 AM, Eric Sandeen wrote:
>>> +
>>> + if (f2fs_test_compress_extension(F2FS_CTX_INFO(ctx).noextensions,
>>> + F2FS_CTX_INFO(ctx).nocompress_ext_cnt,
>>> + F2FS_CTX_INFO(ctx).extensions,
>>> + F2FS_CTX_INFO(ctx).compress_ext_cnt)) {
>>> + f2fs_err(sbi, "invalid compress or nocompress extension");
>> Can you please describe what is detailed confliction in the log? e.g. new
>> noext conflicts w/ new ext...
> Hmm, let me think about this. I had not noticed it was calling
> f2fs_test_compress_extension 3 times, I wonder if there is a better option.
> I need to understand this approach better. Maybe Hongbo has thoughts.
(taking linux-fsdevel off cc: to reduce noise)
Looking at this some more, I don't think any information is lost with this
change. The messages are exactly the same as they were before, in the error
cases.
(I don't love the "check 3 times" logic, but I guess we have to check internal
ctx consistency, as well as ctx vs. sbi, and sbi vs. ctx).
I think that if you would like to see clearer error messages, that's outside
the scope of the mount API conversion.
(If you have an example of a kernel message difference under this mount API
conversion vs current upstream, I'd be happy to look in more detail.)
Thanks,
-Eric
>>> + return -EINVAL;
>>> + }
>>> + if (f2fs_test_compress_extension(F2FS_CTX_INFO(ctx).noextensions,
>>> + F2FS_CTX_INFO(ctx).nocompress_ext_cnt,
>>> + F2FS_OPTION(sbi).extensions,
>>> + F2FS_OPTION(sbi).compress_ext_cnt)) {
>>> + f2fs_err(sbi, "invalid compress or nocompress extension");
>> Ditto,
>>
>>> + return -EINVAL;
>>> + }
>>> + if (f2fs_test_compress_extension(F2FS_OPTION(sbi).noextensions,
>>> + F2FS_OPTION(sbi).nocompress_ext_cnt,
>>> + F2FS_CTX_INFO(ctx).extensions,
>>> + F2FS_CTX_INFO(ctx).compress_ext_cnt)) {
>>> + f2fs_err(sbi, "invalid compress or nocompress extension");
>> Ditto,
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 5/7] f2fs: separate the options parsing and options checking
On 5/6/25 5:01 PM, Jaegeuk Kim wrote:
>> +static int f2fs_check_opt_consistency(struct fs_context *fc,
>> + struct super_block *sb)
>> +{
>> +struct f2fs_fs_context *ctx = fc->fs_private;
>> +struct f2fs_sb_info *sbi = F2FS_SB(sb);
>> +int err;
>> +
>> +if (ctx_test_opt(ctx, F2FS_MOUNT_NORECOVERY) && !f2fs_readonly(sb))
>> +return -EINVAL;
>> +
>> +if (f2fs_hw_should_discard(sbi) && (ctx->opt_mask & F2FS_MOUNT_DISCARD)
>> +&& !ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
> Applied.
>
>if (f2fs_hw_should_discard(sbi) &&
>(ctx->opt_mask & F2FS_MOUNT_DISCARD) &&
>!ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
>
yes that's nicer
>> +f2fs_warn(sbi, "discard is required for zoned block devices");
>> +return -EINVAL;
>> +}
>> +
>> +if (f2fs_sb_has_device_alias(sbi)) {
> Shouldn't this be?
>
> if (f2fs_sb_has_device_alias(sbi) &&
> !ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
>
Whoops, I don't know how I missed that, or how my testing missed it, sorry.
And maybe it should be later in the function so it doesn't interrupt the=
discard cases.
>> +f2fs_err(sbi, "device aliasing requires extent cache");
>> +return -EINVAL;
>> +}
>> +
>> +if (!f2fs_hw_support_discard(sbi) && (ctx->opt_mask &
>> F2FS_MOUNT_DISCARD)
>> +&& ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
>if (!f2fs_hw_support_discard(sbi) &&
>(ctx->opt_mask & F2FS_MOUNT_DISCARD) &&
>ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/6/25 11:02 AM, Jaegeuk Kim wrote: > On 05/05, Eric Sandeen wrote: >> Hi all - it would be nice to get some review or feedback on this; >> seems that these patches tend to go stale fairly quickly as f2fs >> evolves. :) > > Thank you so much for the work! Let me queue this series into dev-test for > tests. If I find any issue, let me ping to the thread. So, you don't need > to worry about rebasing it. :) Thank you for queuing it, and Hongbo for the original series. Please reach out if you encounter any problems. -Eric > Thanks, > >> >> Thanks, >> -Eric >> ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/6/25 9:56 PM, Eric Sandeen wrote: > On 5/6/25 8:23 PM, Jaegeuk Kim wrote: ... >> What about: >> # mount -o loop,noextent_cache f2fsfile.img mnt >> >> In this case, 1) ctx_clear_opt(), 2) set_opt() in default_options, >> 3) clear_opt since mask is set? > > Not sure what I'm missing, it seems to work properly here but I haven't > pulled your (slightly) modified patches yet: > > # mount -o loop,extent_cache f2fsfile.img mnt > # mount | grep -wo extent_cache > extent_cache > # umount mnt > > # mount -o loop,noextent_cache f2fsfile.img mnt > # mount | grep -wo noextent_cache > noextent_cache > # > > this looks right? > > I'll check your tree tomorrow, though it doesn't sound like you made many > changes. Hmm, I checked tonight and I see the same (correct?) behavior in your tree. >> And, device_aliasing check is still failing, since it does not understand >> test_opt(). Probably it's the only case? Again, in your tree (I had to use a git version of f2fs-tools to make device aliasing work - maybe time for a release?) ;) # mkfs.ext4 /dev/vdc # mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb # mount -o noextent_cache /dev/vdb mnt # dmesg | tail -n 1 [ 581.924604] F2FS-fs (vdb): device aliasing requires extent cache # mount -o extent_cache /dev/vdb mnt # mount | grep -wo extent_cache extent_cache # Maybe you can show me exactly what's not working for you? -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/6/25 7:35 PM, Jaegeuk Kim wrote: > Hmm, I had to drop the series at the moment, since it seems needing more > work to deal with default_options(), which breaks my device setup. > For example, set_opt(sbi, READ_EXTENT_CACHE) in default_options is not > propagating > to the below logics. In this case, do we need ctx_set_opt() if user doesn't > set? Hm, can you describe the test or environment that fails for you? (I'm afraid that I may not have all the right hardware to test everything, so may have missed some cases) However, from a quick test here, a loopback mount of an f2fs image file does set extent_cache properly, so maybe I don't understand the problem: # mount -o loop f2fsfile.img mnt # mount | grep -o extent_cache extent_cache # I'm happy to try to look into it though. Maybe you can put the patches back on a temporary branch for me to pull and test? Thanks, - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion
On 5/6/25 8:23 PM, Jaegeuk Kim wrote: > On 05/06, Eric Sandeen wrote: >> On 5/6/25 7:35 PM, Jaegeuk Kim wrote: >>> Hmm, I had to drop the series at the moment, since it seems needing more >>> work to deal with default_options(), which breaks my device setup. >>> For example, set_opt(sbi, READ_EXTENT_CACHE) in default_options is not >>> propagating >>> to the below logics. In this case, do we need ctx_set_opt() if user doesn't >>> set? >> >> Hm, can you describe the test or environment that fails for you? >> (I'm afraid that I may not have all the right hardware to test everything, >> so may have missed some cases) >> >> However, from a quick test here, a loopback mount of an f2fs image file does >> set extent_cache properly, so maybe I don't understand the problem: >> >> # mount -o loop f2fsfile.img mnt >> # mount | grep -o extent_cache >> extent_cache >> # >> >> I'm happy to try to look into it though. Maybe you can put the patches >> back on a temporary branch for me to pull and test? > > Thank you! I pushed here the last version. > > https://github.com/jaegeuk/f2fs/commits/mount/ > > What about: > # mount -o loop,noextent_cache f2fsfile.img mnt > > In this case, 1) ctx_clear_opt(), 2) set_opt() in default_options, > 3) clear_opt since mask is set? Not sure what I'm missing, it seems to work properly here but I haven't pulled your (slightly) modified patches yet: # mount -o loop,extent_cache f2fsfile.img mnt # mount | grep -wo extent_cache extent_cache # umount mnt # mount -o loop,noextent_cache f2fsfile.img mnt # mount | grep -wo noextent_cache noextent_cache # this looks right? I'll check your tree tomorrow, though it doesn't sound like you made many changes. > And, device_aliasing check is still failing, since it does not understand > test_opt(). Probably it's the only case? I think you can blame me, not Hongbo on that one ;) - I will look into it. -Eric >> >> Thanks, >> - Eric >> > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 7/7] f2fs: switch to the new mount api
On 5/11/25 10:43 PM, Chao Yu wrote:
> On 5/8/25 23:59, Eric Sandeen wrote:
>> On 5/8/25 4:19 AM, Chao Yu wrote:
>>>> @@ -2645,21 +2603,11 @@ static int f2fs_remount(struct
>>>> super_block *sb, int *flags, char *data)
>>>>
>>>> default_options(sbi, true);
>>>>
>>>> - memset(&fc, 0, sizeof(fc)); - memset(&ctx, 0, sizeof(ctx));
>>>> - fc.fs_private = &ctx; - fc.purpose =
>>>> FS_CONTEXT_FOR_RECONFIGURE; - -/* parse mount options */ -
>>>> err = parse_options(&fc, data); - if (err) - goto
>>>> restore_opts;
>>> There is a retry flow during f2fs_fill_super(), I intenionally
>>> inject a fault into f2fs_fill_super() to trigger the retry flow,
>>> it turns out that mount option may be missed w/ below testcase:
>>
>> I never did understand that retry logic (introduced in ed2e621a95d
>> long ago). What errors does it expect to be able to retry, with
>> success?
>
> IIRC, it will retry mount if there is recovery failure due to
> inconsistent metadata.
Sure, I just wonder what would cause inconsistent metadata to become consistent
after 1 retry ...
>>
>> Anyway ...
>>
>> Can you show me (as a patch) exactly what you did to trigger the
>> retry, just so we are looking at the same thing?
>
> You can try this?
Ok, thanks!
-Eric
> --- fs/f2fs/super.c | 6 ++ 1 file changed, 6 insertions(+)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index
> 0ee783224953..10f0e66059f8 100644 --- a/fs/f2fs/super.c +++ b/fs/
> f2fs/super.c @@ -5066,6 +5066,12 @@ static int
> f2fs_fill_super(struct super_block *sb, struct fs_context *fc) goto
> reset_checkpoint; }
>
> + if (retry_cnt) { + err = -EIO; + skip_recovery =
> true; + goto
> free_meta; + } + /* recover fsynced data */ if (!test_opt(sbi,
> DISABLE_ROLL_FORWARD) && !test_opt(sbi, NORECOVERY)) {
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 7/7] f2fs: switch to the new mount api
On 5/14/25 10:30 AM, Jaegeuk Kim wrote: > Hi, Hongbo, > > It seems we're getting more issues in the patch set. May I ask for some > help sending the new patch series having all the fixes that I made as well > as addressing the concerns? You can get the patches from [1]. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/log/?h=dev-test Apologies for being a little absent, some urgent things came up internally for me at work and I've had less time for this. I'm planning to get back to it! It's been a long thread, perhaps we could summarize the remaining questions and concerns? And I'm sorry for the errors that got through, I really thought my testing was fairly exhaustive, but it appears that I missed several cases. (To be fair, f2fs has far more mount options than any other filesystem, and combining that with on-disk feature variants, it is a very big test matrix.) Thanks, -Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
