On 9/17/2020 10:52 AM, Michal Hocko wrote:
On Thu 17-09-20 10:27:16, Vijay Balakrishna wrote:
On 9/17/2020 2:28 AM, Michal Hocko wrote:
On Wed 16-09-20 23:39:39, Vijay Balakrishna wrote:
set_recommended_min_free_kbytes need to honor min_free_kbytes set by the
user. Post start-of-day THP enable or memory hotplug operations can
lose user specified min_free_kbytes, in particular when it is higher than
calculated recommended value.
I was about to recommend a more detailed explanation when I have
realized that this patch is not really needed after all. Unless I am
missing something.
init_per_zone_wmark_min ignores the newly calculated min_free_kbytes if
it is lower than user_min_free_kbytes. So calculated min_free_kbytes >=
user_min_free_kbytes.
Except for value clamping when the value is reduced and this likely
needs fixing. But set_recommended_min_free_kbytes should be fine.
IIUC, after start-of-day if a user performs
- THP disable
- modifies min_free_bytes
- THP enable
above sequence currently wouldn't result in calling init_per_zone_wmark_min.
I will not, but why do you think this matters? All we should care about
is that auto-tuning shouldn't reduce user provided value [1] and that
the memory hotplug should be consistent with the boot time heuristic.
init_per_zone_wmark_min should make sure that the user value is not
reduced and thp heuristic makes sure it will not reduce this value.
So the property should be transitive with the existing code (modulo the
problem I have highlighted).
[1] one could argue that it shouldn't even increase the value strictly
speaking because an admin might have a very good reason to decrease the
value but this has never been the semantic and changing it now might be
problematic
I made an attempt to address Kirill A. Shutemov's comment. And incrased
min_free_kbytes to see the issue in my testing and attempted a fix. I'm
ok leaving as it is. Do not want introduce any changes that may cause
regression.
Thanks,
Vijay
