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