Re: [PATCH v2] mm: vmscan: abort proactive reclaim early when freezing for suspend

[Michal Hocko <mhocko@suse.com>]

On Wed 15-07-26 19:47:37, Oleg Nesterov wrote:
> Hi Michal,
> 
> On 07/15, Michal Hocko wrote:
> >
> > Oleg you haven't been CCed from the top of the thread but TL;DR is that
> > we need to freeze a user task performing pro-active memory reclaim that
> > might take long.  Currently we are calling signal_pending to bail out
> > but that leaks EINTR which is bad as freezing task has a side effect of
> > terminating it.
> 
> Sorry, I guess I don't understand the problem...

the problem is that `echo $AMOUNT > $CGROUP/memory.reclaim' can take
quite some time and therefore block freezer. Appart from Richard's fix
to bail out on pending_signal deep in the MGLRU reclaim we have this
signal_pending in user_proactive_reclaim which would leak EINTR if the
source of the signal was freezer. This is IMHO undesirable. The proposed
change below tries to address that problem.

Makes more sense now?

> 
> Yes, unlike EINTR, ERESTARTSYS should not leak to the caller (just in case,
> debugger can see this error code).

Yes, I guess this is a general constrain to any signal_pending() caller.

> But I am sure you know this. And the patch looks fine to me.
> 
> Oleg.

Thanks!
> 
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 35c3bb15ae96..336af76d2a28 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -7909,8 +7909,13 @@ int user_proactive_reclaim(char *buf,
> >  		unsigned long batch_size = (nr_to_reclaim - nr_reclaimed) / 4;
> >  		unsigned long reclaimed;
> >  
> > +		/*
> > +		 * Make sure that we enter freezer without delaying much and
> > +		 * leaking EINTR when that happens but also take care of 
> > +		 * fatal signals to terminate.
> > +		 */
> >  		if (signal_pending(current))
> > -			return -EINTR;
> > +			return -ERESTARTSYS;
> >  
> >  		/*
> >  		 * This is the final attempt, drain percpu lru caches in the
> > -- 
> > Michal Hocko
> > SUSE Labs
> > 

-- 
Michal Hocko
SUSE Labs