Re: [PATCH v2 1/2] mm: huge_memory: refactor thpsize_shmem_enabled_store() with sysfs_match_string()

[Lorenzo Stoakes]

On Mon, May 18, 2026 at 02:02:08AM +0000, ranxiaokai627@xxxxxxx wrote:
> >On Wed, May 13, 2026 at 09:45:07AM +0000, ranxiaokai627@xxxxxxx wrote:
> >> From: Ran Xiaokai <ran.xiaokai@xxxxxxxxxx>
> >>
> >> Inspired by commit 82d9ff648c6c ("mm: huge_memory: refactor
> >> anon_enabled_store() with set_anon_enabled_mode()"), refactor
> >> thpsize_shmem_enabled_store() using sysfs_match_string().
> >> This eliminates the duplicated spin_lock/unlock(), set/clear_bit(),
> >> calls across all branches, reducing code duplication.
> >
> >>  static ssize_t thpsize_shmem_enabled_show(struct kobject *kobj,
> >>  					  struct kobj_attribute *attr, char *buf)
> >>  {
> >> @@ -5551,57 +5574,42 @@ static ssize_t thpsize_shmem_enabled_store(struct kobject *kobj,
> >>  					   const char *buf, size_t count)
> >>  {
> >>  	int order = to_thpsize(kobj)->order;
> >> +	int mode, m;
> >
> >I hate single letter variable names (yes I see set_anon_enabled_mode() has it, I
> >should have reviewed against that, oops :).
> >
> >Please give it its type of enum huge_shmem_enalbed_mode.
>
> Hi, Lorenzo,
>
> int main()
> {
>     enum huge_shmem_enabled_mode mode = 0;
>     const char* type_name = _Generic((mode),
>         int: "signed int",
>         unsigned int: "unsigned int",
>         default: "other"
>     );
>     printf("enum huge_shmem_enabled_mode underlying type is: %s\n", type_name);
>     return 0;
> }
>  ./a.out
> enum huge_shmem_enabled_mode underlying type is: unsigned int
>
> This will cause the following check to always evaluate to false and
> does not validate input parameters like echo "xxx" > .../hugepages-xxkB/shmem_enable
> if (mode < 0)
> 	return mode;
>
> So i think we should keep mode as integer.
>
> >> +	mode = sysfs_match_string(huge_shmem_enabled_mode_strings, buf);
> >> +	if (mode < 0)
> >> +		return -EINVAL;

OK fair enough. The C type system strikes again :)

>

Cheers, Lorenzo