lkml.org 
[lkml]   [2014]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Patch] x86,mm: check freeze request in page fault handler
On Mon 11-08-14 17:53:55, Cong Wang wrote:
> When a process triggers a page fault and kernel keeps
> trying to retry the fault, there is no chance for this process
> to be frozen, so the freeze request will always be pending.

The retry cannot happen indefinitely, no?

Besides that the patch is broken in at least 2 ways. You are not
releasing mmap_sem and this will break memcg OOM killer handling.

If a memcg is under OOM (because of hard limit) then try_charge
calls mem_cgroup_oom which marks the current task with OOM
information. Notably takes a reference to memcg->css. The charge fail
will then gets up the pagefault stack until we get to mm_fault_error
where you put the task into freezer and then returns without
pagefault_out_of_memory which would handle memcg specific parts in
mem_cgroup_oom_synchronize. If the task wakes up and the page fault
retry succeeds (because some charges were released in the meantime) then
you leak a reference to memcg->css.

Besides that the whole change would need a better justification. Why
other archs do not need this?

> This patch lets the page fault handler check pending
> freeze request and freeze current process if so.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> arch/x86/mm/fault.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index a241946..ad9728a 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -14,6 +14,7 @@
> #include <linux/hugetlb.h> /* hstate_index_to_shift */
> #include <linux/prefetch.h> /* prefetchw */
> #include <linux/context_tracking.h> /* exception_enter(), ... */
> +#include <linux/freezer.h> /* try_to_freeze() */
>
> #include <asm/traps.h> /* dotraplinkage, ... */
> #include <asm/pgalloc.h> /* pgd_*(), ... */
> @@ -885,6 +886,9 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code,
> up_read(&current->mm->mmap_sem);
> no_context(regs, error_code, address, 0, 0);
> return;
> + } else if (signal_pending(current) && (error_code & PF_USER)) {
> + if (try_to_freeze())
> + return;
> }
>
> if (fault & VM_FAULT_OOM) {
> --
> 1.8.3.1
>

--
Michal Hocko
SUSE Labs


\
 
 \ /
  Last update: 2014-08-12 14:21    [W:0.108 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site