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 <t...@linutronix.de>
> Cc: Ingo Molnar <mi...@redhat.com>
> Cc: David Rientjes <rient...@google.com>
> Cc: Michal Hocko <mho...@suse.cz>
> Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
> Cc: Tejun Heo <t...@kernel.org>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Signed-off-by: Cong Wang <xiyou.wangc...@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(¤t->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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
