Re: [PATCH -v3 5/5] oom, PM: make OOM detection in the freezer path raceless

[Michal Hocko]

On Sat 10-01-15 14:43:22, Tejun Heo wrote:
> On Fri, Jan 09, 2015 at 12:05:55PM +0100, Michal Hocko wrote:
> ...
> > @@ -142,7 +118,6 @@ static bool check_frozen_processes(void)
> >  int freeze_processes(void)
> >  {
> >  	int error;
> > -	int oom_kills_saved;
> >  
> >  	error = __usermodehelper_disable(UMH_FREEZING);
> >  	if (error)
> > @@ -157,29 +132,22 @@ int freeze_processes(void)
> >  	pm_wakeup_clear();
> >  	pr_info("Freezing user space processes ... ");
> >  	pm_freezing = true;
> > -	oom_kills_saved = oom_kills_count();
> >  	error = try_to_freeze_tasks(true);
> >  	if (!error) {
> >  		__usermodehelper_set_disable_depth(UMH_DISABLED);
> > -		oom_killer_disable();
> > -
> > -		/*
> > -		 * There might have been an OOM kill while we were
> > -		 * freezing tasks and the killed task might be still
> > -		 * on the way out so we have to double check for race.
> > -		 */
> > -		if (oom_kills_count() != oom_kills_saved &&
> > -		    !check_frozen_processes()) {
> > -			__usermodehelper_set_disable_depth(UMH_ENABLED);
> > -			pr_cont("OOM in progress.");
> > -			error = -EBUSY;
> > -		} else {
> > -			pr_cont("done.");
> > -		}
> > +		pr_cont("done.");
> >  	}
> >  	pr_cont("\n");
> >  	BUG_ON(in_atomic());
> >  
> > +	/*
> > +	 * Now that the whole userspace is frozen we need to disbale
> > +	 * the OOM killer to disallow any further interference with
> > +	 * killable tasks.
> > +	 */
> > +	if (!error && !oom_killer_disable())
> 
> So, previously, oom killer was disabled at the top of
> freeze_kernel_threads(), right?  I think that was the better spot to
> do that.  We don't want to disable oom killer before the system is
> just about to enter total quiescence which is freeze_kernel_threads().
> We want to delay this as long as possible.  Let's please disable oom
> killing in at the top of freeze_kernel_threads() and re-enable at the
> bottom of thaw_kernel_threads().

Yes I had it this way but it didn't work out because thaw_kernel_threads
is not called on the resume because it is only used as a fail
path when kernel threads freezing fails. I would rather keep the
enabling/disabling points as we had them. This is less risky IMHO.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/