On Fri, 13 May 2011 20:42:11 +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? Currently, swap toke priority is only decreased at page fault
> path. Then, if the process sleep immediately after to grab swap
> token, their swap token priority never be decreased. That makes
> obviously undesired result.
>
> This patch implement very poor (and lightweight) priority decay
> mechanism. It only be affect to the above corner case and doesn't
> change swap tendency workload performance (eg multi process qsbench
> load)
>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
But...
> ---
> include/trace/events/vmscan.h | 12 ++++++++----
> mm/thrash.c | 5 ++++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 1798e0c..ba18137 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -366,9 +366,10 @@ DEFINE_EVENT_CONDITION(put_swap_token_template, disable_swap_token,
>
> TRACE_EVENT_CONDITION(update_swap_token_priority,
> TP_PROTO(struct mm_struct *mm,
> - unsigned int old_prio),
> + unsigned int old_prio,
> + struct mm_struct *swap_token_mm),
>
> - TP_ARGS(mm, old_prio),
> + TP_ARGS(mm, old_prio, swap_token_mm),
>
> TP_CONDITION(mm->token_priority != old_prio),
>
> @@ -376,16 +377,19 @@ TRACE_EVENT_CONDITION(update_swap_token_priority,
> __field(struct mm_struct*, mm)
> __field(unsigned int, old_prio)
> __field(unsigned int, new_prio)
> + __field(unsigned int, token_prio)
> ),
>
> TP_fast_assign(
> __entry->mm = mm;
> __entry->old_prio = old_prio;
> __entry->new_prio = mm->token_priority;
> + __entry->token_prio = swap_token_mm ? swap_token_mm->token_priority : 0;
> ),
>
> - TP_printk("mm=%p old_prio=%u new_prio=%u",
> - __entry->mm, __entry->old_prio, __entry->new_prio)
> + TP_printk("mm=%p old_prio=%u new_prio=%u token_prio=%u",
> + __entry->mm, __entry->old_prio, __entry->new_prio,
> + __entry->token_prio)
> );
>
> #endif /* _TRACE_VMSCAN_H */
> diff --git a/mm/thrash.c b/mm/thrash.c
> index 14c6c9f..0c4f0a8 100644
> --- a/mm/thrash.c
> +++ b/mm/thrash.c
> @@ -47,6 +47,9 @@ void grab_swap_token(struct mm_struct *mm)
> if (!swap_token_mm)
> goto replace_token;
>
> + if (!(global_faults & 0xff))
> + mm->token_priority /= 2;
> +
I personally don't like this kind of checking counter with mask.
Hmm. How about this ?
==
#define TOKEN_AGE_MASK ~(0xff)
/*
* If current global_fault is in different age from previous global_fault,
* Aging priority and starts new era.
*/
if ((mm->faultstamp & TOKEN_AGE_MASK) != (global_faults & MM_TOKEN_MASK))
mm->token_priority /= 2;
==
But I'm not sure 0xff is a proper value or not...
Thanks,
-Kame
--
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>