On Wed, 20 Jan 2021 00:06:14 +0000
HORIGUCHI NAOYA(堀口 直也) <naoya.horiguchi@xxxxxxx> wrote:
> You don't have to add these header info into patch description because
> git can get them from email headers.
> # scripts/checkpatch.pl shows some warning due to these.
yes, i will change this. Thanks.
> > if (fail || tk->addr == -EFAULT) {
> > - pr_err("Memory failure: %#lx: forcibly killing %s:%d because of failure to unmap corrupted page\n",
> > - pfn, tk->tsk->comm, tk->tsk->pid);
> > - do_send_sig_info(SIGKILL, SEND_SIG_PRIV,
> > + if (tk->tsk == current) {
> > + pr_err("Memory failure: %#lx: forcibly killing %s:%d because of failure to unmap corrupted page\n",
> > + pfn, tk->tsk->comm, tk->tsk->pid);
> > + do_send_sig_info(SIGKILL, SEND_SIG_PRIV,
> > tk->tsk, PIDTYPE_PID);
> > + } else if (kill_proc(tk, pfn, flags) < 0)
> > + pr_err("Memory failure: %#lx: Cannot send advisory machine check signal to %s:%d\n",
> > + pfn, tk->tsk->comm, tk->tsk->pid);
>
> This seems to change (maybe beyond the intent of this patch) the behavior of
> action optional events, i.e. without this patch all processes on to_kill list
> receive SIGKILL. but with patch SIGKILL is sent only to current (if it
> happens to be linked to to_kill list) and any other processes on the list
> receive SIGBUSs. Any justification on this change?
>
> This code path is for failure in error handling, where we can't do anything
> more. So I think that just killing all affected processes with SIGKILL as
> we do now is the expected behavior.
you are right, I have some misunderstanding here, we need to keep the original code. Thanks for correction
> > /*
> > @@ -457,8 +464,6 @@ static struct task_struct *task_early_kill(struct task_struct *tsk,
> > */
> > if (tsk->mm == current->mm)
> > return current;
> > - else
> > - return NULL;
>
> Please update the comment above this function about action required case.
yes, I will add some comments here! I will submit a v3 patch.
Thank you!
--
Best Regards!
Aili Yao
