Messages in this thread | | | Date | Mon, 26 Feb 2024 12:49:51 +0100 | Subject | Re: [PATCH] mm, thp: display [never] for defrag when THP is set to never | From | David Hildenbrand <> |
| |
On 26.02.24 12:48, David Hildenbrand wrote: > On 26.02.24 12:22, Liu Song wrote: >> >> 在 2024/2/26 16:54, David Hildenbrand 写道: >>> On 23.02.24 12:01, Liu Song wrote: >>>> >>>> 在 2024/2/22 20:14, David Hildenbrand 写道: >>>>> On 22.02.24 12:53, Yuanhe Shu wrote: >>>>>> When transparent_hugepage is set to never by cmdline or echo, defrag >>>>>> still show what it used to be and can be modified which makes user >>>>>> confusing whether defrag would take effect. >>>>>> >>>>>> Actually if transparent_hugepage is set to never, defrag will not take >>>>>> effect. Just Display never and remain unchangeable to for defrag when >>>>>> transparent_hugepage is set to never. >>>>>> >>>>>> Suggested-by: Liu Song <liusong@linux.alibaba.com> >>>>>> Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com> >>>>>> --- >>>>> >>>>> No, I don't think we want such a dependency between both options. >>>>> >>>>> You might just end up breaking existing scripts (enable defrag before >>>>> enabling THP) for no good reason. >>>>> >>>> In certain situations where khugepaged_thread is NULL, it would be more >>>> reasonable for the value of >>>> /sys/kernel/mm/transparent_hugepage/khugepaged/defrag to be 0. The patch >>>> should include a fix for this case. >>> >>> Why? >>> >>> We have a bunch of THP toggles. They reside in >>> "/sys/kernel/mm/transparent_hugepage/", indicating that they are THP >>> specific. >>> >>> Some of them are only in effect if some other toggles are set. >>> >>> That is very common practice. >>> >>> If you think something could be confusing, maybe clarify the doc? I >>> don't immediately see why any code changes are required, really. >> >> We should explain this in the documentation, but to be honest, many >> people don't read the documentation, and even after we explicitly >> disable THP with transparent_hugepage=never, khugepaged/defrag is still >> set to 1, and users come asking why it's still defragging. We can't >> expect all users to have technical expertise, or to diligently read >> through the documentation; it would obviously be best if we could avoid >> user confusion altogether. > > Then tell your users if a feature is enabled
s/enabled/disabled/ s/1on1/101/
Time for a break :D
Anyhow, there has to be a pretty good reason to change the semantics of these toggles that have been around forever. "users could be confused" is a not sufficient.
-- Cheers,
David / dhildenb
| |