From: Punit Agrawal <punit.agrawal@arm.com>
To: Tyler Baicar <tbaicar@codeaurora.org>
Cc: catalin.marinas@arm.com, will.deacon@arm.com,
mark.rutland@arm.com, james.morse@arm.com,
akpm@linux-foundation.org, zjzhang@codeaurora.org,
sandeepa.s.prabhu@gmail.com, shijie.huang@arm.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: hwpoison: add VM_FAULT_HWPOISON[_LARGE] handling
Date: Fri, 03 Feb 2017 16:17:17 +0000 [thread overview]
Message-ID: <877f57qmeq.fsf@e105922-lin.cambridge.arm.com> (raw)
In-Reply-To: <1485987318-9145-1-git-send-email-tbaicar@codeaurora.org> (Tyler Baicar's message of "Wed, 1 Feb 2017 15:15:18 -0700")
Tyler Baicar <tbaicar@codeaurora.org> writes:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>
> Add VM_FAULT_HWPOISON[_LARGE] handling to the arm64 page fault
> handler. Handling of VM_FAULT_HWPOISON[_LARGE] is very similar
> to VM_FAULT_OOM, the only difference is that a different si_code
> (BUS_MCEERR_AR) is passed to user space and si_addr_lsb field is
> initialized.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> ---
> arch/arm64/mm/fault.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
[...]
> @@ -426,7 +439,17 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
> */
> sig = SIGBUS;
> code = BUS_ADRERR;
> - } else {
> + }
> +#ifdef CONFIG_MEMORY_FAILURE
> + else if (fault & (VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) {
Please add spaces around '|'.
> + pr_err(
> + "Killing %s:%d due to hardware memory corruption fault at %lx\n",
> + tsk->comm, tsk->pid, addr);
The message is misleading as we're not really killing a task but
delivering a signal (SIGBUS) which might not always lead to the receiver
being killed.
But considering that we don't print any message for the other faults,
I'd prefer that we drop this pr_err.
> + sig = SIGBUS;
> + code = BUS_MCEERR_AR;
> + }
> +#endif
Although to get a HWPOISON fault CONFIG_MEMORY_FAILURE is needed, the
handling seems safe even when it is not enabled. Can the ifdeffery be
dropped?
Also, I was wondering how this code was tested? Did you by any chance
try using hwpoison inject debugfs interface?
Thanks,
Punit
> + else {
> /*
> * Something tried to access memory that isn't in our memory
> * map.
> @@ -436,7 +459,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
> SEGV_ACCERR : SEGV_MAPERR;
> }
>
> - __do_user_fault(tsk, addr, esr, sig, code, regs);
> + __do_user_fault(tsk, addr, esr, sig, code, regs, fault);
> return 0;
>
> no_context:
next prev parent reply other threads:[~2017-02-03 16:17 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-01 22:15 [PATCH] arm64: hwpoison: add VM_FAULT_HWPOISON[_LARGE] handling Tyler Baicar
2017-02-03 16:17 ` Punit Agrawal [this message]
[not found] ` <f92b8ab6-3728-e038-e3c8-bcaa6491075e@codeaurora.org>
2017-02-07 17:19 ` Punit Agrawal
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=877f57qmeq.fsf@e105922-lin.cambridge.arm.com \
--to=punit.agrawal@arm.com \
--cc=akpm@linux-foundation.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=sandeepa.s.prabhu@gmail.com \
--cc=shijie.huang@arm.com \
--cc=tbaicar@codeaurora.org \
--cc=will.deacon@arm.com \
--cc=zjzhang@codeaurora.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;
as well as URLs for NNTP newsgroup(s).