Thread (4 messages) 4 messages, 3 authors, 2021-09-17

Re: [PATCH v2] vmpressure: wake up work only when there is registration event

From: yong w <hidden>
Date: 2021-09-17 15:38:50
Also in: linux-mm, lkml

Michal Hocko [off-list ref] 于2021年9月15日周三 下午8:42写道:
On Tue 14-09-21 09:05:51, yongw.pur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
quoted
From: wangyong <redacted>

Use the global variable num_events to record the number of vmpressure
events registered by the system, and wake up work only when there is
registration event.
Usually, the vmpressure event is not registered in the system, this patch
can avoid waking up work and doing nothing.
I have asked in the previous version and this changelog doesn't that
explain again. Why don't you simply bail out early in vmpressure()
entry?
Thanks for your reply.
Because the else branch will modify the socket_pressure, and will not wake up
the work. It is necessary to judge the tree parameters at the same
time, like this
if (tree && !static_branch_unlikely(&num_events))
    return;
It's not good to judge the tree twice parameters in the function.
quoted
Test with 5.14.0-rc5-next-20210813 on x86_64 4G ram.
Consume cgroup memory until it is about to be reclaimed, then execute
"perf stat -I 2000 malloc.out" command to trigger memory reclamation
and get performance results.
The context-switches is reduced by about 20 times.
Is this test somewhere available so that it can be reproduced by
others. Also while the number of context switches can be an interesting
it is not really clear from this evaluation whether that actually
matters or not. E.g. what does an increase of task-clock and twice as
many instructions recorded tell us?
The test program is a simple malloc  process, which allocate memory
and fill some data.
I think it may be that more instructions can be executed per unit time.
quoted
unpatched:
Average of 10 test results
582.4674048   task-clock(msec)
19910.8               context-switches
0             cpu-migrations
1292.9                page-faults
414784733.1   cycles
quoted
<not supported>       stalled-cycles-frontend
<not supported>       stalled-cycles-backend
Why is this a part of the data?
quoted
580070698.4   instructions
125572244.7   branches
2073541.2     branch-misses

patched
Average of 10 test results
973.6174796   task-clock(msec)
988.6         context-switches
0             cpu-migrations
1785.2                page-faults
772883602.4   cycles
<not supported>       stalled-cycles-frontend
<not supported>       stalled-cycles-backend
1360280911    instructions
290519434.9   branches
3378378.2     branch-misses

Tested-by: Zeal Robot <redacted>
Signed-off-by: wangyong <redacted>
---
[...]
quoted
@@ -272,6 +277,9 @@ void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
              return;

      if (tree) {
+             if (!static_branch_unlikely(&num_events))
+                     return;
We usually hide the change behind a static inline helper (e.g.
vmpressure_disabled()). I would also put it to the beginning of
vmpressure or put an explanation why it makes sense only in this branch.
--
Because only this branch needs to wake up work.
Yes, static inline helper is more easier to read and understand.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help