From: Oscar Salvador <osalvador@suse.de>
To: Aili Yao <yaoaili@kingsoft.com>
Cc: naoya.horiguchi@nec.com, linux-mm@kvack.org, YANGFENG1@kingsoft.com
Subject: Re: [PATCH v5] mm,hwpoison: send SIGBUS to PF_MCE_EARLY processes on action required events
Date: Thu, 21 Jan 2021 11:03:05 +0100 [thread overview]
Message-ID: <20210121100257.GA11685@linux> (raw)
In-Reply-To: <20210120162422.0ed3dd56.yaoaili@kingsoft.com>
On Wed, Jan 20, 2021 at 04:24:22PM +0800, Aili Yao wrote:
> When a memory uncorrected error is triggered by process who accessed
> the address with error, It's Action Required Case for only current
> process which triggered this; This Action Required case means Action
> optional to other process who share the same page. Usually killing
> current process will be sufficient, other processes sharing the same
> page will get be signaled when they really touch the poisoned page.
>
> But there is another scenario that other processes
> sharing the same page want to be signaled early with PF_MCE_EARLY set,
> In this case, we should get them into kill list and signal
> BUS_MCEERR_AO to them.
>
> So in this patch, task_early_kill will check current process if
> force_early is set, and if not current,the code will fallback to
> find_early_kill_thread() to check if there is PF_MCE_EARLY process
> who cares the error.
>
> In kill_proc(), BUS_MCEERR_AR is only send to current, other processes in
> kill list will be signaled with BUS_MCEERR_AO.
>
> Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
Looks good to me, a few nits below.
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> @@ -243,9 +243,12 @@ static int kill_proc(struct to_kill *tk, unsigned long pfn, int flags)
> pfn, t->comm, t->pid);
>
> if (flags & MF_ACTION_REQUIRED) {
> - WARN_ON_ONCE(t != current);
> - ret = force_sig_mceerr(BUS_MCEERR_AR,
> + if (tk->tsk == current)
You can re-use "t" here.
> + ret = force_sig_mceerr(BUS_MCEERR_AR,
> (void __user *)tk->addr, addr_lsb);
> + else
> + ret = send_sig_mceerr(BUS_MCEERR_AO, (void __user *)tk->addr,
> + addr_lsb, t);
I would place a brief comment above explaining why we are sending BUS_MCEER_AO
to non-current tasks.
E.g: "Signal other processes sharing the page if they have PF_MCE_EARLY set"
> @@ -457,8 +463,6 @@ static struct task_struct *task_early_kill(struct task_struct *tsk,
> */
> if (tsk->mm == current->mm)
> return current;
> - else
> - return NULL;
if (force_early && task->mm == current->mm)
return current;
--
Oscar Salvador
SUSE L3
next prev parent reply other threads:[~2021-01-21 10:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-20 8:24 [PATCH v5] mm,hwpoison: send SIGBUS to PF_MCE_EARLY processes on action required events Aili Yao
2021-01-21 10:03 ` Oscar Salvador [this message]
2021-01-22 5:13 ` Aili Yao
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=20210121100257.GA11685@linux \
--to=osalvador@suse.de \
--cc=YANGFENG1@kingsoft.com \
--cc=linux-mm@kvack.org \
--cc=naoya.horiguchi@nec.com \
--cc=yaoaili@kingsoft.com \
/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).