[PATCH 0/4 -v2] OOM vs. freezer interaction fixes

[Michal Hocko]

Hi Andrew, Rafael,

this has been originally discussed here [1] and previously posted here [2].
I have updated patches according to feedback from Oleg.

The first and third patch are regression fixes and they are a stable
material IMO. The second and fourth patch are simple cleanups.

The 1st patch is fixing a regression introduced in 3.3 since when OOM
killer is not able to kill any frozen task and live lock as a result.
The fix gets us back to the 3.2. As it turned out during the discussion [3]
this was still not 100% sufficient and that's why we need the 3rd patch.

I was thinking about the proper 1st vs. 3rd patch ordering because
the 1st patch basically opens a race window considerably reduced by the
later patch. This path is hard to do completely race free without a complete
synchronization of OOM path (including the allocator) and freezer which is not
worth the trouble.

Original patch from Cong Wang has covered this by checking
cgroup_freezing(current) in __refrigarator path [4]. But this approach
still suffers from OOM vs. PM freezer interaction (OOM killer would
still live lock waiting for a PM frozen task this time).

So I think the most straight forward way is to address only OOM vs.
frozen task interaction in the first patch, mark it for stable 3.3+ and
leave the race to a separate follow up patch which is applicable to
stable 3.2+ (before a3201227f803 made it inefficient).

Switching 1st and 3rd patches would make some sense as well but then
it might end up even more confusing because we would be fixing a
non-existent issue in upstream first...

Cong Wang (2):
freezer: Do not freeze tasks killed by OOM killer
freezer: remove obsolete comments in __thaw_task()

Michal Hocko (2):
OOM, PM: OOM killed task shouldn't escape PM suspend
PM: convert do_each_thread to for_each_process_thread

And diffstat says:
include/linux/oom.h | 3 +++
kernel/freezer.c | 9 +++------
kernel/power/process.c | 47 ++++++++++++++++++++++++++++++++++++++---------
mm/oom_kill.c | 17 +++++++++++++++++
mm/page_alloc.c | 8 ++++++++
5 files changed, 69 insertions(+), 15 deletions(-)

---
[1] http://marc.info/?l=linux-kernel&m=140986986423092
[2] http://marc.info/?l=linux-mm&m=141277728508500&w=2
[3] http://marc.info/?l=linux-kernel&m=141074263721166
[4] http://marc.info/?l=linux-kernel&m=140986986423092
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Michal Hocko]

From: Cong Wang <***@gmail.com>

Since f660daac474c6f (oom: thaw threads if oom killed thread is frozen
before deferring) OOM killer relies on being able to thaw a frozen task
to handle OOM situation but a3201227f803 (freezer: make freezing() test
freeze conditions in effect instead of TIF_FREEZE) has reorganized the
code and stopped clearing freeze flag in __thaw_task. This means that
the target task only wakes up and goes into the fridge again because the
freezing condition hasn't changed for it. This reintroduces the bug
fixed by f660daac474c6f.

Fix the issue by checking for TIF_MEMDIE thread flag in
freezing_slow_path and exclude the task from freezing completely. If a
task was already frozen it would get woken by __thaw_task from OOM killer
and get out of freezer after rechecking freezing().

Changes since v1
- put TIF_MEMDIE check into freezing_slowpath rather than in __refrigerator
as per Oleg
- return __thaw_task into oom_scan_process_thread because
oom_kill_process will not wake task in the fridge because it is
sleeping uninterruptible

[***@suse.cz: rewrote the changelog]
Fixes: a3201227f803 (freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE)
Cc: ***@vger.kernel.org # 3.3+
Cc: David Rientjes <***@google.com>
Cc: Michal Hocko <***@suse.cz>
Cc: "Rafael J. Wysocki" <***@rjwysocki.net>
Cc: Tejun Heo <***@kernel.org>
Cc: Andrew Morton <***@linux-foundation.org>
Signed-off-by: Cong Wang <***@gmail.com>
Signed-off-by: Michal Hocko <***@suse.cz>
Acked-by: Oleg Nesterov <***@redhat.com>
---
kernel/freezer.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/freezer.c b/kernel/freezer.c
index aa6a8aadb911..8f9279b9c6d7 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -42,6 +42,9 @@ bool freezing_slow_path(struct task_struct *p)
if (p->flags & (PF_NOFREEZE | PF_SUSPEND_TASK))
return false;

+ if (test_thread_flag(TIF_MEMDIE))
+ return false;
+
if (pm_nosig_freezing || cgroup_freezing(p))
return true;

[Rafael J. Wysocki]

Post by Michal Hocko
Since f660daac474c6f (oom: thaw threads if oom killed thread is frozen
before deferring) OOM killer relies on being able to thaw a frozen task
to handle OOM situation but a3201227f803 (freezer: make freezing() test
freeze conditions in effect instead of TIF_FREEZE) has reorganized the
code and stopped clearing freeze flag in __thaw_task. This means that
the target task only wakes up and goes into the fridge again because the
freezing condition hasn't changed for it. This reintroduces the bug
fixed by f660daac474c6f.
Fix the issue by checking for TIF_MEMDIE thread flag in
freezing_slow_path and exclude the task from freezing completely. If a
task was already frozen it would get woken by __thaw_task from OOM killer
and get out of freezer after rechecking freezing().
Changes since v1
- put TIF_MEMDIE check into freezing_slowpath rather than in __refrigerator
as per Oleg
- return __thaw_task into oom_scan_process_thread because
oom_kill_process will not wake task in the fridge because it is
sleeping uninterruptible
Fixes: a3201227f803 (freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE)

ACK

Post by Michal Hocko
---
kernel/freezer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index aa6a8aadb911..8f9279b9c6d7 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -42,6 +42,9 @@ bool freezing_slow_path(struct task_struct *p)
if (p->flags & (PF_NOFREEZE | PF_SUSPEND_TASK))
return false;
+ if (test_thread_flag(TIF_MEMDIE))
+ return false;
+
if (pm_nosig_freezing || cgroup_freezing(p))
return true;

[Michal Hocko]

From: Cong Wang <***@gmail.com>

__thaw_task() no longer clears frozen flag since commit a3201227f803
(freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE).

Cc: David Rientjes <***@google.com>
Cc: "Rafael J. Wysocki" <***@rjwysocki.net>
Cc: Tejun Heo <***@kernel.org>
Cc: Andrew Morton <***@linux-foundation.org>
Reviewed-by: Michal Hocko <***@suse.cz>
Signed-off-by: Cong Wang <***@gmail.com>
---
kernel/freezer.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/kernel/freezer.c b/kernel/freezer.c
index 8f9279b9c6d7..a8900a3bc27a 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -150,12 +150,6 @@ void __thaw_task(struct task_struct *p)
{
unsigned long flags;

- /*
- * Clear freezing and kick @p if FROZEN. Clearing is guaranteed to
- * be visible to @p as waking up implies wmb. Waking up inside
- * freezer_lock also prevents wakeups from leaking outside
- * refrigerator.
- */
spin_lock_irqsave(&freezer_lock, flags);
if (frozen(p))
wake_up_process(p);

[Rafael J. Wysocki]

Post by Michal Hocko
__thaw_task() no longer clears frozen flag since commit a3201227f803
(freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE).

ACK

Post by Michal Hocko
---
kernel/freezer.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 8f9279b9c6d7..a8900a3bc27a 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -150,12 +150,6 @@ void __thaw_task(struct task_struct *p)
{
unsigned long flags;
- /*
- * freezer_lock also prevents wakeups from leaking outside
- * refrigerator.
- */
spin_lock_irqsave(&freezer_lock, flags);
if (frozen(p))
wake_up_process(p);

[Michal Hocko]

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.

Fixes: f660daac474c6f (oom: thaw threads if oom killed thread is frozen before deferring)
Cc: Cong Wang <***@gmail.com>
Cc: Rafael J. Wysocki <***@rjwysocki.net>
Cc: Tejun Heo <***@kernel.org>
Cc: David Rientjes <***@google.com>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@vger.kernel.org # 3.2+
Signed-off-by: Michal Hocko <***@suse.cz>
---
include/linux/oom.h | 3 +++
kernel/power/process.c | 31 ++++++++++++++++++++++++++++++-
mm/oom_kill.c | 17 +++++++++++++++++
mm/page_alloc.c | 8 ++++++++
4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
index 647395a1a550..e8d6e1058723 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -50,6 +50,9 @@ static inline bool oom_task_origin(const struct task_struct *p)
extern unsigned long 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 void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
unsigned int points, unsigned long totalpages,
struct mem_cgroup *memcg, nodemask_t *nodemask,
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 4ee194eb524b..a397fa161d11 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -118,6 +118,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)
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }
+out_loop:
+ read_unlock(&tasklist_lock);
+
+ if (error) {
+ __usermodehelper_set_disable_depth(UMH_ENABLED);
+ printk("OOM in progress.");
+ goto done;
+ }
+ }
+ printk("done.");
}
+done:
printk("\n");
BUG_ON(in_atomic());

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index bbf405a3a18f..5340f6b91312 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -404,6 +404,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))
/*
* Must be called while holding a reference to p, which will be released upon
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb573b10af12..efccbbadd7c9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2286,6 +2286,14 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
}

/*
+ * 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
* we're still under heavy pressure.

[Rafael J. Wysocki]

Post by Michal Hocko
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.
Fixes: f660daac474c6f (oom: thaw threads if oom killed thread is frozen before deferring)
---
include/linux/oom.h | 3 +++
kernel/power/process.c | 31 ++++++++++++++++++++++++++++++-
mm/oom_kill.c | 17 +++++++++++++++++
mm/page_alloc.c | 8 ++++++++
4 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 647395a1a550..e8d6e1058723 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -50,6 +50,9 @@ static inline bool oom_task_origin(const struct task_struct *p)
extern unsigned long 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 void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
unsigned int points, unsigned long totalpages,
struct mem_cgroup *memcg, nodemask_t *nodemask,
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 4ee194eb524b..a397fa161d11 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -118,6 +118,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)
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }

Well, it looks like this will work here too:

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
error = -EBUSY;
break;
}

or I am helplessly misreading the code.

Post by Michal Hocko
+ read_unlock(&tasklist_lock);
+
+ if (error) {
+ __usermodehelper_set_disable_depth(UMH_ENABLED);
+ printk("OOM in progress.");
+ goto done;
+ }
+ }
+ printk("done.");
}
printk("\n");
BUG_ON(in_atomic());
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index bbf405a3a18f..5340f6b91312 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -404,6 +404,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))
/*
* Must be called while holding a reference to p, which will be released upon
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb573b10af12..efccbbadd7c9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2286,6 +2286,14 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
}
/*
+ * 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
* we're still under heavy pressure.

[Michal Hocko]

On Tue 21-10-14 14:09:27, Rafael J. Wysocki wrote:
[...]

Post by Michal Hocko

Post by Michal Hocko
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
error = -EBUSY;
break;
}
or I am helplessly misreading the code.

break will not work because for_each_process_thread is a double loop.
Except for that the negated condition is OK as well. I can change that
if you prefer.

Post by Michal Hocko

Post by Michal Hocko
+ read_unlock(&tasklist_lock);
+
+ if (error) {
+ __usermodehelper_set_disable_depth(UMH_ENABLED);
+ printk("OOM in progress.");
+ goto done;
+ }
+ }
+ printk("done.");
}
printk("\n");
BUG_ON(in_atomic());

[Rafael J. Wysocki]

Post by Michal Hocko
[...]

Post by Michal Hocko

Post by Michal Hocko
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
error = -EBUSY;
break;
}
or I am helplessly misreading the code.

break will not work because for_each_process_thread is a double loop.

I see. In that case I'd do:

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {

read_unlock(&tasklist_lock);

__usermodehelper_set_disable_depth(UMH_ENABLED);
printk("OOM in progress.");
error = -EBUSY;
goto done;
}

to avoid adding the new label that looks odd.

[Michal Hocko]

Post by Michal Hocko

Post by Michal Hocko
[...]

Post by Michal Hocko

Post by Michal Hocko
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
error = -EBUSY;
break;
}
or I am helplessly misreading the code.

break will not work because for_each_process_thread is a double loop.

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
read_unlock(&tasklist_lock);
__usermodehelper_set_disable_depth(UMH_ENABLED);
printk("OOM in progress.");
error = -EBUSY;
goto done;
}
to avoid adding the new label that looks odd.

OK, incremental diff on top. I will post the complete patch if you are
happier with this change
---
diff --git a/kernel/power/process.c b/kernel/power/process.c
index a397fa161d11..7a37cf3eb1a2 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -108,6 +108,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.
* The current thread will not be frozen. The same process that calls
@@ -143,25 +165,12 @@ int freeze_processes(void)
* 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) {
- struct task_struct *g, *p;
-
- read_lock(&tasklist_lock);
- for_each_process_thread(g, p) {
- if (p == current || freezer_should_skip(p) ||
- frozen(p))
- continue;
- error = -EBUSY;
- goto out_loop;
- }
-out_loop:
- read_unlock(&tasklist_lock);
-
- if (error) {
- __usermodehelper_set_disable_depth(UMH_ENABLED);
- printk("OOM in progress.");
- goto done;
- }
+ 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.");
}

[Rafael J. Wysocki]

Post by Michal Hocko

Post by Michal Hocko

Post by Michal Hocko
[...]

Post by Michal Hocko

Post by Michal Hocko
@@ -131,12 +132,40 @@ 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) {
+ struct task_struct *g, *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, p) {
+ if (p == current || freezer_should_skip(p) ||
+ frozen(p))
+ continue;
+ error = -EBUSY;
+ goto out_loop;
+ }

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
error = -EBUSY;
break;
}
or I am helplessly misreading the code.

break will not work because for_each_process_thread is a double loop.

for_each_process_thread(g, p)
if (p != current && !frozen(p) &&
!freezer_should_skip(p)) {
read_unlock(&tasklist_lock);
__usermodehelper_set_disable_depth(UMH_ENABLED);
printk("OOM in progress.");
error = -EBUSY;
goto done;
}
to avoid adding the new label that looks odd.

OK, incremental diff on top. I will post the complete patch if you are
happier with this change

Yes, I am.

Post by Michal Hocko
---
diff --git a/kernel/power/process.c b/kernel/power/process.c
index a397fa161d11..7a37cf3eb1a2 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -108,6 +108,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;
+ }
+ }
+ read_unlock(&tasklist_lock);
+
+ return ret;
+}
+
/**
* freeze_processes - Signal user space processes to enter the refrigerator.
* The current thread will not be frozen. The same process that calls
@@ -143,25 +165,12 @@ int freeze_processes(void)
* 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) {
- struct task_struct *g, *p;
-
- read_lock(&tasklist_lock);
- for_each_process_thread(g, p) {
- if (p == current || freezer_should_skip(p) ||
- frozen(p))
- continue;
- error = -EBUSY;
- goto out_loop;
- }
- read_unlock(&tasklist_lock);
-
- if (error) {
- __usermodehelper_set_disable_depth(UMH_ENABLED);
- printk("OOM in progress.");
- goto done;
- }
+ 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.");
}

[Michal Hocko]

[...]

Post by Rafael J. Wysocki

Post by Michal Hocko
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 <***@suse.cz>
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

Fixes: f660daac474c6f (oom: thaw threads if oom killed thread is frozen before deferring)
Cc: Cong Wang <***@gmail.com>
Cc: Rafael J. Wysocki <***@rjwysocki.net>
Cc: Tejun Heo <***@kernel.org>
Cc: David Rientjes <***@google.com>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@vger.kernel.org # 3.2+
Signed-off-by: Michal Hocko <***@suse.cz>
---
include/linux/oom.h | 3 +++
kernel/power/process.c | 40 +++++++++++++++++++++++++++++++++++++++-
mm/oom_kill.c | 17 +++++++++++++++++
mm/page_alloc.c | 8 ++++++++
4 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
index 647395a1a550..e8d6e1058723 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -50,6 +50,9 @@ static inline bool oom_task_origin(const struct task_struct *p)
extern unsigned long 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 void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
unsigned int points, unsigned long totalpages,
struct mem_cgroup *memcg, nodemask_t *nodemask,
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 4ee194eb524b..7a37cf3eb1a2 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -108,6 +108,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.
* The current thread will not be frozen. The same process that calls
@@ -118,6 +140,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)
@@ -131,12 +154,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());

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index bbf405a3a18f..5340f6b91312 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -404,6 +404,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))
/*
* Must be called while holding a reference to p, which will be released upon
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb573b10af12..22f1929469ec 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2286,6 +2286,14 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
}

/*
+ * 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
* we're still under heavy pressure.

[Rafael J. Wysocki]

Post by Michal Hocko
[...]

Post by Rafael J. Wysocki

Post by Michal Hocko
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
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 ***@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"***@kvack.org"> ***@kvack.org </a>

[Michal Hocko]

Post by Rafael J. Wysocki

Post by Michal Hocko
[...]

Post by Rafael J. Wysocki

Post by Michal Hocko
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
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.

Post by Rafael J. Wysocki
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;
- }
- }
+ 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.");
}
printk("\n");
BUG_ON(in_atomic());
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
see: http://www.linux-mm.org/ .

[Rafael J. Wysocki]

Post by Michal Hocko

Post by Rafael J. Wysocki

Post by Michal Hocko
[...]

Post by Rafael J. Wysocki

Post by Michal Hocko
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
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.

I'll apply it then, thanks!

[Michal Hocko]

as per 0c740d0afc3b (introduce for_each_thread() to replace the buggy
while_each_thread()) get rid of do_each_thread { } while_each_thread()
construct and replace it by a more error prone for_each_thread.

This patch doesn't introduce any user visible change.

Suggested-by: Oleg Nesterov <***@redhat.com>
Signed-off-by: Michal Hocko <***@suse.cz>
---
kernel/power/process.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/power/process.c b/kernel/power/process.c
index a397fa161d11..7fd7b72554fe 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -46,13 +46,13 @@ static int try_to_freeze_tasks(bool user_only)
while (true) {
todo = 0;
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p == current || !freeze_task(p))
continue;

if (!freezer_should_skip(p))
todo++;
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);

if (!user_only) {
@@ -93,11 +93,11 @@ static int try_to_freeze_tasks(bool user_only)

if (!wakeup) {
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p != current && !freezer_should_skip(p)
&& freezing(p) && !frozen(p))
sched_show_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
}
} else {
@@ -219,11 +219,11 @@ void thaw_processes(void)
thaw_workqueues();

read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
/* No other threads should have PF_SUSPEND_TASK set */
WARN_ON((p != curr) && (p->flags & PF_SUSPEND_TASK));
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);

WARN_ON(!(curr->flags & PF_SUSPEND_TASK));
@@ -246,10 +246,10 @@ void thaw_kernel_threads(void)
thaw_workqueues();

read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p->flags & (PF_KTHREAD | PF_WQ_WORKER))
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);

schedule();

[Rafael J. Wysocki]

Post by Michal Hocko
as per 0c740d0afc3b (introduce for_each_thread() to replace the buggy
while_each_thread()) get rid of do_each_thread { } while_each_thread()
construct and replace it by a more error prone for_each_thread.
This patch doesn't introduce any user visible change.

ACK

Or do you want me to handle this series?

Post by Michal Hocko
---
kernel/power/process.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index a397fa161d11..7fd7b72554fe 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -46,13 +46,13 @@ static int try_to_freeze_tasks(bool user_only)
while (true) {
todo = 0;
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p == current || !freeze_task(p))
continue;
if (!freezer_should_skip(p))
todo++;
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
if (!user_only) {
@@ -93,11 +93,11 @@ static int try_to_freeze_tasks(bool user_only)
if (!wakeup) {
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p != current && !freezer_should_skip(p)
&& freezing(p) && !frozen(p))
sched_show_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
}
} else {
@@ -219,11 +219,11 @@ void thaw_processes(void)
thaw_workqueues();
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
/* No other threads should have PF_SUSPEND_TASK set */
WARN_ON((p != curr) && (p->flags & PF_SUSPEND_TASK));
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
WARN_ON(!(curr->flags & PF_SUSPEND_TASK));
@@ -246,10 +246,10 @@ void thaw_kernel_threads(void)
thaw_workqueues();
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p->flags & (PF_KTHREAD | PF_WQ_WORKER))
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
schedule();

[Michal Hocko]

Post by Rafael J. Wysocki

Post by Michal Hocko
as per 0c740d0afc3b (introduce for_each_thread() to replace the buggy
while_each_thread()) get rid of do_each_thread { } while_each_thread()
construct and replace it by a more error prone for_each_thread.
This patch doesn't introduce any user visible change.

ACK
Or do you want me to handle this series?

I don't know, I hoped either you or Andrew to pick it up.

Post by Rafael J. Wysocki

Post by Michal Hocko
---
kernel/power/process.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index a397fa161d11..7fd7b72554fe 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -46,13 +46,13 @@ static int try_to_freeze_tasks(bool user_only)
while (true) {
todo = 0;
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p == current || !freeze_task(p))
continue;
if (!freezer_should_skip(p))
todo++;
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
if (!user_only) {
@@ -93,11 +93,11 @@ static int try_to_freeze_tasks(bool user_only)
if (!wakeup) {
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p != current && !freezer_should_skip(p)
&& freezing(p) && !frozen(p))
sched_show_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
}
} else {
@@ -219,11 +219,11 @@ void thaw_processes(void)
thaw_workqueues();
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
/* No other threads should have PF_SUSPEND_TASK set */
WARN_ON((p != curr) && (p->flags & PF_SUSPEND_TASK));
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
WARN_ON(!(curr->flags & PF_SUSPEND_TASK));
@@ -246,10 +246,10 @@ void thaw_kernel_threads(void)
thaw_workqueues();
read_lock(&tasklist_lock);
- do_each_thread(g, p) {
+ for_each_process_thread(g, p) {
if (p->flags & (PF_KTHREAD | PF_WQ_WORKER))
__thaw_task(p);
- } while_each_thread(g, p);
+ }
read_unlock(&tasklist_lock);
schedule();

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[Rafael J. Wysocki]

Post by Michal Hocko

Post by Rafael J. Wysocki

Post by Michal Hocko
as per 0c740d0afc3b (introduce for_each_thread() to replace the buggy
while_each_thread()) get rid of do_each_thread { } while_each_thread()
construct and replace it by a more error prone for_each_thread.
This patch doesn't introduce any user visible change.

ACK
Or do you want me to handle this series?

I don't know, I hoped either you or Andrew to pick it up.

OK, I will then.