memcg: minor ->mm_owner fix/cleanups
cover memcg: minor ->mm_owner fix/cleanups
Andrew,
You applied the last (4th) memcg-kill-config_mm_owner.patch, but other patches in this thread were ignored. Hopefully thi is because I sent them chaotically.
Let me resend 1-3 in case you missed them. Acked by Michal.
Oleg.
init/main.c | 1 - kernel/exit.c | 18 +++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-)
PATCH RESEND 1/3 memcg: mm_update_next_owner() should skip kthreads
"Search through everything else" in mm_update_next_owner() can hit a kthread which adopted this "mm" via use_mm(), it should not be used as mm->owner. Add the PF_KTHREAD check.
While at it, change this code to use for_each_process_thread() instead of deprecated do_each_thread/while_each_thread.
--- kernel/exit.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)
kernel/exit.c +4 -6
@@ -356,14 +356,12 @@ retry: }/* -* Search through everything else. We should not get -* here often +* Search through everything else, we should not get here often. */ - do_each_thread(g, c) { -if (c->mm == mm) +for_each_process_thread (g, c) { +if (!(c->flags &PF_KTHREAD ) && c->mm == mm) goto assign_new_owner; - }while_each_thread (g, c); - + }read_unlock (&tasklist_lock);/* * We found no owner yet mm_users > 1: this implies that we are
Signature · 1 line
1.5.5.1
PATCH RESEND 3/3 memcg: kill start_kernel()->mm_init_owner(&init_mm)
In reply to Oleg NesterovRemove start_kernel()->mm_init_owner(&init_mm, &init_task).
This doesn't really hurt but unnecessary and misleading. init_task is the "swapper" thread == current, its ->mm is always NULL. And init_mm can only be used as ->active_mm, not as ->mm.
mm_init_owner() has a single caller with this patch, perhaps it should die. mm_init() can initialize ->owner under #ifdef.
--- init/main.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
init/main.c +0 -1
@@ -507,7 +507,6 @@ asmlinkage void __init start_kernel(void)page_address_init ();pr_notice ("%s" , linux_banner);setup_arch (&command_line); -mm_init_owner (&init_mm, &init_task);mm_init_cpumask (&init_mm);setup_command_line (command_line);setup_nr_cpu_ids ();
Signature · 1 line
1.5.5.1
PATCH RESEND 2/3 memcg: optimize the "Search everything else" loop in mm_update_next_owner()
In reply to Oleg Nesterovfor_each_process_thread() is sub-optimal. All threads share the same ->mm, we can swicth to the next process once we found a thread with ->mm != NULL and ->mm != mm.
--- kernel/exit.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
kernel/exit.c +9 -3
@@ -358,9 +358,15 @@ retry:/* * Search through everything else, we should not get here often. */ -for_each_process_thread (g, c) { -if (!(c->flags &PF_KTHREAD ) && c->mm == mm) - goto assign_new_owner; +for_each_process (g) { +if (g->flags &PF_KTHREAD ) +continue ; +for_each_thread (g, c) { +if (c->mm == mm) + goto assign_new_owner; +if (c->mm) +break ; + } }read_unlock (&tasklist_lock);/*
Signature · 1 line
1.5.5.1
Quoted 5 lines · Oleg Nesterov
Andrew,
You applied the last (4th) memcg-kill-config_mm_owner.patch, but other patches in this thread were ignored. Hopefully thi is because I sent them chaotically.
All of them are queued AFAICS. I do not know where those emails are archived so i've bounced them to you off-list.
Quoted 8 lines
Let me resend 1-3 in case you missed them. Acked by Michal.
Oleg.
init/main.c | 1 - kernel/exit.c | 18 +++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-)
Signature · 2 lines
Michal Hocko SUSE Labs