Re: [f2fs-dev] [PATCH 2/7] f2fs: move the option parser into handle_mount_opt

[Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 05/07, Eric Sandeen wrote:
> On 5/7/25 6:26 AM, Chao Yu wrote:
> > On 4/20/25 23:25, Eric Sandeen wrote:
> >> From: Hongbo Li <lihongbo22@huawei.com>
> >>
> >> 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-chao@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?

Yeah, agreed we'd better applying the type change later, once mount API is
successfully landed. Chao, let's keep checking any missing cases. :)

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