Re: [PATCH v2] mm,hwpoison:Differentiate Action Required error in current and non current process

[Aili Yao <yaoaili@kingsoft.com>]

On Wed, 20 Jan 2021 00:06:14 +0000
HORIGUCHI NAOYA(堀口　直也) <naoya.horiguchi@nec.com> 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