lkml.org 
[lkml]   [2011]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/3] vmscan: implement swap token priority aging
On Thu, 19 May 2011 11:34:15 +0900
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> 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.


> 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?

`global_faults' and `last_aging' could be made static local in
grab_swap_token().

> 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 :(


\
 
 \ /
  Last update: 2011-05-20 21:33    [W:0.155 / U:1.672 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site