Messages in this thread Patch in this message | | | Date | Tue, 12 Aug 2014 17:47:02 -0700 | Subject | Re: [Patch] x86,mm: check freeze request in page fault handler | From | Cong Wang <> |
| |
On Tue, Aug 12, 2014 at 5:09 AM, Michal Hocko <mhocko@suse.cz> wrote: > 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.
Ah, it's my bad to miss the up_read().
> > 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.
OK, I thought skipping pagefault_out_of_memory() is safe, you are right here, thanks for explanation.
> > Besides that the whole change would need a better justification. Why > other archs do not need this? >
They probably need this as well, just that I only have x86 to care and test. :)
Does the following updated patch make any sense to you? If not, I will just drop it.
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index a241946..8c0a7d8 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_*(), ... */ @@ -904,6 +905,9 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code, * oom-killed): */ pagefault_out_of_memory(); + + if (signal_pending(current)) + try_to_freeze(); } else { if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON| VM_FAULT_HWPOISON_LARGE))
| |