Re: [PATCH v7 3/7] mm: check fatal signal pending of target process

[Minchan Kim]

On Fri, Mar 06, 2020 at 11:22:07AM +0100, Vlastimil Babka wrote:
> On 3/2/20 8:36 PM, Minchan Kim wrote:
> > Bail out to prevent unnecessary CPU overhead if target process has
> > pending fatal signal during (MADV_COLD|MADV_PAGEOUT) operation.
> > 
> > Reviewed-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> > Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> 
> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
> 
> Nit below:
> 
> > ---
> >  mm/madvise.c | 29 +++++++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 349473fc6683..6543f2bfc3d8 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -36,6 +36,7 @@
> >  struct madvise_walk_private {
> >  	struct mmu_gather *tlb;
> >  	bool pageout;
> > +	struct task_struct *target_task;
> >  };
> >  
> >  /*
> > @@ -316,6 +317,10 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
> >  	if (fatal_signal_pending(current))
> >  		return -EINTR;
> >  
> > +	if (private->target_task &&
> > +			fatal_signal_pending(private->target_task))
> > +		return -EINTR;
> 
> With madvise(2) private->target_task will be current, thus current will be
> tested twice. Not wrong, but maybe add a "private->target_task != current"
> condition?

It was in old series but removed because reviewer(IIRC, suren) wanted it.
I am not strong preference either way. Since you said it's nit and
considering other reviewer wanted to remove it, I will not change
further.

Thanks!