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

[Breno Leitao <leitao@debian.org>]

On Wed, May 13, 2026 at 08:25:58AM +0000, ranxiaokai627@163.com wrote:
> >On 5/12/26 8:05 PM, ranxiaokai627@163.com wrote:
> >> From: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> >>
> >> 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.
> >>
> >> Tested with selftests ./run_kselftest.sh -t mm:ksft_thp.sh,
> >> all test cases passed.
> >>
> >> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> >> ---
> >
> >Thanks for doing this.

Ack. I appreciate you taking the initiative to convert the remaining
instances.

> >Moreover, I think we can follow commit 82d9ff648c6c's approach: if
> >nothing changed, we don't need to call start_stop_khugepaged().
>
> Thanks for the review.
> Yes, indeed, We can further optimize this by following the approach
> in commit 82d9ff648c6c.

Agreed, that would be a good improvement.

> It seems odd to still call set_recommended_min_free_kbytes() when the
> mode hasn't changed. Maybe this is to handle the case where the user
> modified /proc/sys/vm/min_free_kbytes in this timewindow?

You're right that it appears unusual. The rationale is to avoid breaking
userspace for applications that may be 'triggering' this behavior by
rewriting the same value to sysfs. While this isn't an expected use
case, we need to maintain backward compatibility.

The discussion around this took place here, if you're interested in the
context:
https://lore.kernel.org/all/ec07d7f0-cad4-4d9b-8e40-d4ded8170340@lucifer.local/

Please CC me on the next revision.

Thanks,
--breno