From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com [209.85.217.181]) by kanga.kvack.org (Postfix) with ESMTP id 4F0FB6B007B for ; Wed, 22 Oct 2014 10:22:29 -0400 (EDT) Received: by mail-lb0-f181.google.com with SMTP id l4so2879262lbv.40 for ; Wed, 22 Oct 2014 07:22:28 -0700 (PDT) Received: from mx2.suse.de (cantor2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id f1si23576177lam.8.2014.10.22.07.22.25 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 22 Oct 2014 07:22:26 -0700 (PDT) Date: Wed, 22 Oct 2014 16:22:26 +0200 From: Michal Hocko Subject: Re: [PATCH 3/4] OOM, PM: OOM killed task shouldn't escape PM suspend Message-ID: <20141022142226.GC30802@dhcp22.suse.cz> References: <1413876435-11720-1-git-send-email-mhocko@suse.cz> <4766859.KSKPTm3b0x@vostro.rjw.lan> <20141021142939.GG9415@dhcp22.suse.cz> <3987583.vdsuvlAsHc@vostro.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3987583.vdsuvlAsHc@vostro.rjw.lan> Sender: owner-linux-mm@kvack.org List-ID: To: "Rafael J. Wysocki" Cc: Andrew Morton , Cong Wang , David Rientjes , Tejun Heo , Oleg Nesterov , LKML , linux-mm@kvack.org, Linux PM list On Wed 22-10-14 16:39:12, Rafael J. Wysocki wrote: > On Tuesday, October 21, 2014 04:29:39 PM Michal Hocko wrote: > > On Tue 21-10-14 16:41:07, Rafael J. Wysocki wrote: > > > On Tuesday, October 21, 2014 04:11:59 PM Michal Hocko wrote: > > [...] > > > > OK, incremental diff on top. I will post the complete patch if you are > > > > happier with this change > > > > > > Yes, I am. > > --- > > From 9ab46fe539cded8e7b6425b2cd23ba9184002fde Mon Sep 17 00:00:00 2001 > > From: Michal Hocko > > Date: Mon, 20 Oct 2014 18:12:32 +0200 > > Subject: [PATCH -v2] OOM, PM: OOM killed task shouldn't escape PM suspend > > > > PM freezer relies on having all tasks frozen by the time devices are > > getting frozen so that no task will touch them while they are getting > > frozen. But OOM killer is allowed to kill an already frozen task in > > order to handle OOM situtation. In order to protect from late wake ups > > OOM killer is disabled after all tasks are frozen. This, however, still > > keeps a window open when a killed task didn't manage to die by the time > > freeze_processes finishes. > > > > Reduce the race window by checking all tasks after OOM killer has been > > disabled. This is still not race free completely unfortunately because > > oom_killer_disable cannot stop an already ongoing OOM killer so a task > > might still wake up from the fridge and get killed without > > freeze_processes noticing. Full synchronization of OOM and freezer is, > > however, too heavy weight for this highly unlikely case. > > > > Introduce and check oom_kills counter which gets incremented early when > > the allocator enters __alloc_pages_may_oom path and only check all the > > tasks if the counter changes during the freezing attempt. The counter > > is updated so early to reduce the race window since allocator checked > > oom_killer_disabled which is set by PM-freezing code. A false positive > > will push the PM-freezer into a slow path but that is not a big deal. > > > > Changes since v1 > > - push the re-check loop out of freeze_processes into > > check_frozen_processes and invert the condition to make the code more > > readable as per Rafael > > I've applied that along with the rest of the series, but what about the > following cleanup patch on top of it? Sure, looks good to me. > > Rafael > > > --- > kernel/power/process.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > Index: linux-pm/kernel/power/process.c > =================================================================== > --- linux-pm.orig/kernel/power/process.c > +++ linux-pm/kernel/power/process.c > @@ -108,25 +108,27 @@ static int try_to_freeze_tasks(bool user > return todo ? -EBUSY : 0; > } > > +static bool __check_frozen_processes(void) > +{ > + struct task_struct *g, *p; > + > + for_each_process_thread(g, p) > + if (p != current && !freezer_should_skip(p) && !frozen(p)) > + return false; > + > + return true; > +} > + > /* > * Returns true if all freezable tasks (except for current) are frozen already > */ > static bool check_frozen_processes(void) > { > - struct task_struct *g, *p; > - bool ret = true; > + bool ret; > > read_lock(&tasklist_lock); > - for_each_process_thread(g, p) { > - if (p != current && !freezer_should_skip(p) && > - !frozen(p)) { > - ret = false; > - goto done; > - } > - } > -done: > + ret = __check_frozen_processes(); > read_unlock(&tasklist_lock); > - > return ret; > } > > @@ -167,15 +169,14 @@ int freeze_processes(void) > * on the way out so we have to double check for race. > */ > if (oom_kills_count() != oom_kills_saved && > - !check_frozen_processes()) { > + !check_frozen_processes()) { > __usermodehelper_set_disable_depth(UMH_ENABLED); > printk("OOM in progress."); > error = -EBUSY; > - goto done; > + } else { > + printk("done."); > } > - printk("done."); > } > -done: > printk("\n"); > BUG_ON(in_atomic()); > > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org