Skip to content

Commit

Permalink
x86: finish user fault error path with fatal signal
Browse files Browse the repository at this point in the history
The x86 fault handler bails in the middle of error handling when the
task has a fatal signal pending.  For a subsequent patch this is a
problem in OOM situations because it relies on pagefault_out_of_memory()
being called even when the task has been killed, to perform proper
per-task OOM state unwinding.

Shortcutting the fault like this is a rather minor optimization that
saves a few instructions in rare cases.  Just remove it for
user-triggered faults.

Use the opportunity to split the fault retry handling from actual fault
errors and add locking documentation that reads suprisingly similar to
ARM's.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: David Rientjes <rientjes@google.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: azurIt <azurit@pobox.sk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Johannes Weiner authored and Linus Torvalds committed Sep 12, 2013
1 parent 759496b commit 3a13c4d
Showing 1 changed file with 17 additions and 18 deletions.
35 changes: 17 additions & 18 deletions arch/x86/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -842,31 +842,23 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address,
force_sig_info_fault(SIGBUS, code, address, tsk, fault);
}

static noinline int
static noinline void
mm_fault_error(struct pt_regs *regs, unsigned long error_code,
unsigned long address, unsigned int fault)
{
/*
* Pagefault was interrupted by SIGKILL. We have no reason to
* continue pagefault.
*/
if (fatal_signal_pending(current)) {
if (!(fault & VM_FAULT_RETRY))
up_read(&current->mm->mmap_sem);
if (!(error_code & PF_USER))
no_context(regs, error_code, address, 0, 0);
return 1;
if (fatal_signal_pending(current) && !(error_code & PF_USER)) {
up_read(&current->mm->mmap_sem);
no_context(regs, error_code, address, 0, 0);
return;
}
if (!(fault & VM_FAULT_ERROR))
return 0;

if (fault & VM_FAULT_OOM) {
/* Kernel mode? Handle exceptions or die: */
if (!(error_code & PF_USER)) {
up_read(&current->mm->mmap_sem);
no_context(regs, error_code, address,
SIGSEGV, SEGV_MAPERR);
return 1;
return;
}

up_read(&current->mm->mmap_sem);
Expand All @@ -884,7 +876,6 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code,
else
BUG();
}
return 1;
}

static int spurious_fault_check(unsigned long error_code, pte_t *pte)
Expand Down Expand Up @@ -1189,9 +1180,17 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code)
*/
fault = handle_mm_fault(mm, vma, address, flags);

if (unlikely(fault & (VM_FAULT_RETRY|VM_FAULT_ERROR))) {
if (mm_fault_error(regs, error_code, address, fault))
return;
/*
* If we need to retry but a fatal signal is pending, handle the
* signal first. We do not need to release the mmap_sem because it
* would already be released in __lock_page_or_retry in mm/filemap.c.
*/
if (unlikely((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)))
return;

if (unlikely(fault & VM_FAULT_ERROR)) {
mm_fault_error(regs, error_code, address, fault);
return;
}

/*
Expand Down

0 comments on commit 3a13c4d

Please sign in to comment.