Skip to content

Commit

Permalink
OOM, PM: OOM killed task shouldn't escape PM suspend
Browse files Browse the repository at this point in the history
commit 5695be1 upstream.

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

Fixes: f660daa (oom: thaw threads if oom killed thread is frozen before deferring)
Signed-off-by: Michal Hocko <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Zefan Li <[email protected]>

confict:-
  oom.h

Signed-off-by: hurtsky <[email protected]>
Signed-off-by: hemantbeast <[email protected]>
  • Loading branch information
Michal Hocko authored and hemantbeast committed Dec 2, 2016
1 parent 6f0d5ae commit e692d8e
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 1 deletion.
4 changes: 4 additions & 0 deletions include/linux/oom.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ extern int test_set_oom_score_adj(int new_val);

extern unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *memcg,
const nodemask_t *nodemask, unsigned long totalpages);

extern int oom_kills_count(void);
extern void note_oom_kill(void);

extern int try_set_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
extern void clear_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);

Expand Down
40 changes: 39 additions & 1 deletion kernel/power/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,28 @@ static int try_to_freeze_tasks(bool user_only)
return todo ? -EBUSY : 0;
}

/*
* 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;

read_lock(&tasklist_lock);
for_each_process_thread(g, p) {
if (p != current && !freezer_should_skip(p) &&
!frozen(p)) {
ret = false;
goto done;
}
}
done:
read_unlock(&tasklist_lock);

return ret;
}

/**
* freeze_processes - Signal user space processes to enter the refrigerator.
*
Expand All @@ -141,6 +163,7 @@ static int try_to_freeze_tasks(bool user_only)
int freeze_processes(void)
{
int error;
int oom_kills_saved;

error = __usermodehelper_disable(UMH_FREEZING);
if (error)
Expand All @@ -151,12 +174,27 @@ int freeze_processes(void)

printk("Freezing user space processes ... ");
pm_freezing = true;
oom_kills_saved = oom_kills_count();
error = try_to_freeze_tasks(true);
if (!error) {
printk("done.");
__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);
printk("OOM in progress.");
error = -EBUSY;
goto done;
}
printk("done.");
}
done:
printk("\n");
BUG_ON(in_atomic());

Expand Down
17 changes: 17 additions & 0 deletions mm/oom_kill.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,23 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
dump_tasks(memcg, nodemask);
}

/*
* Number of OOM killer invocations (including memcg OOM killer).
* Primarily used by PM freezer to check for potential races with
* OOM killed frozen task.
*/
static atomic_t oom_kills = ATOMIC_INIT(0);

int oom_kills_count(void)
{
return atomic_read(&oom_kills);
}

void note_oom_kill(void)
{
atomic_inc(&oom_kills);
}

#define K(x) ((x) << (PAGE_SHIFT-10))
static void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
unsigned int points, unsigned long totalpages,
Expand Down
8 changes: 8 additions & 0 deletions mm/page_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,14 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
return NULL;
}

/*
* PM-freezer should be notified that there might be an OOM killer on
* its way to kill and wake somebody up. This is too early and we might
* end up not killing anything but false positives are acceptable.
* See freeze_processes.
*/
note_oom_kill();

/*
* Go through the zonelist yet one more time, keep very high watermark
* here, this is only to catch a parallel oom killing, we must fail if
Expand Down

0 comments on commit e692d8e

Please sign in to comment.