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 <[email protected]>
> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
