Hi,
(2011/05/21 4:30), Andrew Morton wrote:
On Thu, 19 May 2011 11:34:15 +0900
KOSAKI Motohiro<kosaki.motohiro@xxxxxxxxxxxxxx> wrote:
While testing for memcg aware swap token, I observed a swap token
was often grabbed an intermittent running process (eg init, auditd)
and they never release a token.
Why?
Some processes (eg init, auditd, audispd) wake up when a process
exiting. And swap token can be get first page-in process when
a process exiting makes no swap token owner. Thus such above
intermittent running process often get a token.
And currently, swap token priority is only decreased at page fault
path. Then, if the process sleep immediately after to grab swap
token, the swap token priority never be decreased. That's obviously
undesirable.
This patch implement very poor (and lightweight) priority aging.
It only be affect to the above corner case and doesn't change swap
tendency workload performance (eg multi process qsbench load)
...
--- a/mm/thrash.c
+++ b/mm/thrash.c
@@ -25,10 +25,13 @@
#include<trace/events/vmscan.h>
+#define TOKEN_AGING_INTERVAL (0xFF)
Needs a comment describing its units and what it does, please.
Sufficient for readers to understand why this value was chosen and what
effect they could expect to see from changing it.
Will do.
static DEFINE_SPINLOCK(swap_token_lock);
struct mm_struct *swap_token_mm;
struct mem_cgroup *swap_token_memcg;
static unsigned int global_faults;
+static unsigned int last_aging;
Is this a good name? Would something like prev_global_faults be better?
Current code is "next-aging = last-aging + 256global-fault". so,
prev_global_faults is slightly misleading to me.
`global_faults' and `last_aging' could be made static local in
grab_swap_token().
OK. even though I don't like static in a function.
void grab_swap_token(struct mm_struct *mm)
{
@@ -47,6 +50,11 @@ void grab_swap_token(struct mm_struct *mm)
if (!swap_token_mm)
goto replace_token;
+ if ((global_faults - last_aging)> TOKEN_AGING_INTERVAL) {
+ swap_token_mm->token_priority /= 2;
+ last_aging = global_faults;
+ }
It's really hard to reverse-engineer the design decisions from the
implementation here, therefore... ?
if (mm == swap_token_mm) {
mm->token_priority += 2;
goto update_priority;
@@ -64,7 +72,7 @@ void grab_swap_token(struct mm_struct *mm)
goto replace_token;
update_priority:
- trace_update_swap_token_priority(mm, old_prio);
+ trace_update_swap_token_priority(mm, old_prio, swap_token_mm);
out:
mm->faultstamp = global_faults;
@@ -80,6 +88,7 @@ replace_token:
trace_replace_swap_token(swap_token_mm, mm);
swap_token_mm = mm;
swap_token_memcg = memcg;
+ last_aging = global_faults;
goto out;
}
In fact all of grab_swap_token() and the thrash-detection code in
general are pretty tricky and unobvious stuff. So we left it
undocumented :(
Well, original swap token (designed by Rik) is pretty straight forward
implementation on the theory. Therefore we didn't need too verbose doc.
The paper give us good well documentation.
# Oh, now http://www.cs.wm.edu/~sjiang/token.pdf is dead link.
# we should fix it to http://www.cse.ohio-state.edu/hpcs/WWW/HTML/publications/papers/TR-05-1.pdf,
# maybe, or do anybody know better url?
But following commit rewrite almost all code. and we lost good documentation.
It's a bit sad.
commit 7602bdf2fd14a40dd9b104e516fdc05e1bd17952
Author: Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxxxx>
Date: Wed Dec 6 20:31:57 2006 -0800
[PATCH] new scheme to preempt swap token
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>