Re: [PATCH V2] mm: compaction: support triggering of proactive compaction by user

[Charan Teja Kalla]

Thanks Nitin for your inputs!!

On 5/26/2021 2:05 AM, Nitin Gupta wrote:
> The proactive compaction[1] gets triggered for every 500msec and run
> compaction on the node for COMPACTION_HPAGE_ORDER (usually order-9)
> pages based on the value set to sysctl.compaction_proactiveness.
> Triggering the compaction for every 500msec in search of
> 
> COMPACTION_HPAGE_ORDER pages is not needed for all applications,
>> especially on the embedded system usecases which may have few MB's of
>> RAM. Enabling the proactive compaction in its state will endup in running
>> almost always on such systems.
>>
> You can disable proactive compaction by setting sysctl.compaction_proactiveness to 0.

Agree. But proactive compaction got its own uses too like it knows when
to stop the compaction, instead of simply doing the full node
compaction, thus we don't want to disable it always.

> 
> 
>> As an example, say app
>> launcher decide to launch the memory heavy application which can be
>> launched fast if it gets more higher order pages thus launcher can prepare
>> the system in advance by triggering the proactive compaction from
>> userspace.
>>
> You can always do: echo 1 > /proc/sys/vm/compact_memory
> On a small system, this should not take much time.

Hmm... With 3GB Snapdragon system, we have observed that write to
compact_memory is taking peak time of 400+msec, could be that
MIGRATE_SYNC on a full node is causing this peak, which is much time.


> 
> Hijacking proactive compaction for one-off compaction (say, before a large app launch)
> does not sound right to me.

Actually we are using the proactive compaction to 'just prepare the
system before asking for huge memory' as compact_memory can take longer
and is not controllable like proactive compaction.

In the V1 of this patch, we actually created a /proc interface(similar
to compact_memory), providing a way to trigger the proactive compaction
from user space. https://lore.kernel.org/patchwork/patch/1417064/. But
since this involved a new /proc interface addition, in V2 we just
implemented an alternative way to it.

Another problem, I think, this patch tried to address is that, in the
existing implementation it is not guaranteed the user set value of
compaction_proactiveness is effective unless atleast
HPAGE_FRAG_CHECK_INTERVAL_MSEC(500msec) is elapsed, Right? Does this
seems correct provided we had given this user interface and can't
specified any where when this value will be effective(where it comes
into effect in the next compact thread wake up for proactive compaction).

Consider the below testcase where a user thinks that the application he
is going to run is performance critical thus decides to do the below steps:
1) Save the present the compaction_proactiveness (Say it is zero thus
disabled)
2) Set the compaction_proactiveness to 100.
3) Allocate memory for the application.
4) Restore the compaction_proactiveness.(set to disabled again)
5) Then proactive compaction is tried to run.

First, Does the user doing the above steps are valid?
If yes, then we should guarantee to the user that proactive compaction
atleast tried to run when the user changed the proactiveness.
If not, I feel, we should document that 'once user changed the
compaction_proactiveness, he need to wait atleast
HPAGE_FRAG_CHECK_INTERVAL_MSEC before considering that the value he
tried to set is effective and proactive compaction tried to run on
that'. Doesn't this seem okay?

--Charan
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project