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