From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH] ARC: Improve handling of fatal signals in do_page_fault()
Date: Fri, 29 Jun 2018 11:20:05 -0700 [thread overview]
Message-ID: <20180629182005.10243-1-abrodkin@synopsys.com> (raw)
This was triggered by investigation of a deadlock after OOM killer invocation,
see [1] for more details.
Looks like our handling of fatal signal in do_page_fault() has some issues:
1. We only want to do special (read "early") handling of fatal signal
if handle_mm_fault() returned VM_FAULT_RETRY so that we don't loop
in retry loop endlessly, otherwise we'll handle that signal normally
on exit from exception handler.
2. up_read() is not needed as indeed it will be done in __lock_page_or_retry()
in mm/filemap.c.
With above comments in mind simplified version should be like that:
------------------------------->8---------------------------
if (fatal_signal_pending(current)
if (fault & VM_FAULT_RETRY)
if (user_mode(regs))
return;
------------------------------->8---------------------------
But looks like there's a room for improvement, see [2].
Instead of proceeding forward and then inevitably hitting retry path we
short-cut right to kernel fix-up code in no_context.
[1] http://lists.infradead.org/pipermail/linux-snps-arc/2018-February/003403.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=746a272e44141af24a02f6c9b0f65f4c4598ed42
Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>
---
arch/arc/mm/fault.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index a0b7bd6d030d..17ed78e2f5eb 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -139,12 +139,16 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
*/
fault = handle_mm_fault(vma, address, flags);
- /* If Pagefault was interrupted by SIGKILL, exit page fault "early" */
+ /* 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(fatal_signal_pending(current))) {
- if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY))
- up_read(&mm->mmap_sem);
- if (user_mode(regs))
+ if (fault & VM_FAULT_RETRY) {
+ if (!user_mode(regs))
+ goto no_context;
return;
+ }
}
perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
--
2.17.1
next reply other threads:[~2018-06-29 18:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-29 18:20 Alexey Brodkin [this message]
2018-07-12 8:32 ` [PATCH] ARC: Improve handling of fatal signals in do_page_fault() Alexey Brodkin
2018-08-01 19:49 ` Vineet Gupta
2018-08-02 7:08 ` Alexey Brodkin
2018-08-02 21:54 ` Vineet Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180629182005.10243-1-abrodkin@synopsys.com \
--to=alexey.brodkin@synopsys.com \
--cc=linux-snps-arc@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox