From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH 3/4] OOM, PM: OOM killed task shouldn't escape PM suspend Date: Wed, 22 Oct 2014 16:39:12 +0200 Message-ID: <3987583.vdsuvlAsHc@vostro.rjw.lan> References: <1413876435-11720-1-git-send-email-mhocko@suse.cz> <4766859.KSKPTm3b0x@vostro.rjw.lan> <20141021142939.GG9415@dhcp22.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: <20141021142939.GG9415@dhcp22.suse.cz> Sender: owner-linux-mm@kvack.org To: Michal Hocko Cc: Andrew Morton , Cong Wang , David Rientjes , Tejun Heo , Oleg Nesterov , LKML , linux-mm@kvack.org, Linux PM list List-Id: linux-pm@vger.kernel.org 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? 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