ACK/CMT: [PATCH] UBUNTU: SAUCE: mm/page_alloc.c: disable memory reclaim watermark boosting by default

[Colin Ian King]

On 27/03/2020 19:26, Sultan Alsawaf wrote:
> On Fri, Mar 27, 2020 at 07:21:30PM +0000, Colin Ian King wrote:
>> On 27/03/2020 18:46, Andrea Righi wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1861359
>>>
>>> High watermark boosting can cause large swap activity under certain
>>> memory intensive workloads, making the system very unresponsive (screen
>>> does not refresh, keyboard not responding, etc.).
>>>
>>> Disable this feature by default to prevent potential large swap
>>> activity.
>>>
>>> Signed-off-by: Sultan Alsawaf <sultan.alsawaf at canonical.com>
>>> Signed-off-by: Andrea Righi <andrea.righi at canonical.com>
>>> ---
>>>  mm/page_alloc.c | 13 -------------
>>>  1 file changed, 13 deletions(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index d387ca74cb5a..7ab52a62c5ef 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -315,20 +315,7 @@ compound_page_dtor * const compound_page_dtors[] = {
>>>  
>>>  int min_free_kbytes = 1024;
>>>  int user_min_free_kbytes = -1;
>>> -#ifdef CONFIG_DISCONTIGMEM
>>> -/*
>>> - * DiscontigMem defines memory ranges as separate pg_data_t even if the ranges
>>> - * are not on separate NUMA nodes. Functionally this works but with
>>> - * watermark_boost_factor, it can reclaim prematurely as the ranges can be
>>> - * quite small. By default, do not boost watermarks on discontigmem as in
>>> - * many cases very high-order allocations like THP are likely to be
>>> - * unsupported and the premature reclaim offsets the advantage of long-term
>>> - * fragmentation avoidance.
>>> - */
>>>  int watermark_boost_factor __read_mostly;
>>> -#else
>>> -int watermark_boost_factor __read_mostly = 15000;
>>> -#endif
>>>  int watermark_scale_factor = 10;
>>>  
>>>  static unsigned long nr_kernel_pages __initdata;
>>>
>>
>> Given that this was introduced by commit 24512228b7a3f412b5 ("mm: do not
>> boost watermarks to avoid fragmentation for the DISCONTIG memory model")
>> for parisc changing this back shouldn't hurt our supported architectures
>> as this returns it back to the original behavior.  This patch looks like
>> a revert of 24512228b7a3f412b5 - could we just revert that commit rather
>> than add a SAUCE patch? It may be worth referencing the problematic commit.
>>
>> Anyhow, the fix is good to me.
>>
>> Acked-by: Colin Ian King <colin.king at canonical.com>
>>
>>
>> -- 
>> kernel-team mailing list
>> kernel-team at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> 
> This patch is different from reverting 24512228b7a3f412b5 because that commit
> sets the default watermark_scale_factor to 0 only for CONFIG_DISCONTIGMEM=y.
> This commit sets the default watermark_scale_factor to 0 for everything.

Ah, I missed that. Thanks for clarifying. :-)

> 
> Also, watermark boosting is a severely flawed "feature" that should not be used
> at all. See my lp comment [1] for more details.

/me nods.

> 
> Sultan
> 
> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1861359/comments/56
>