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 <mhocko@xxxxxxx>
> > 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@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>