linux-kernel - Re: [Patch v4 1/2] freezer: check OOM kill while being frozen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 22 Sep 2014 10:21:30 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Cong Wang <xiyou.wangcong@...il.com>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>, Tejun Heo <tj@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Patch v4 1/2] freezer: check OOM kill while being frozen

[Sorry for the late reply, I was mostly offline throughout last week]

On Tue 16-09-14 15:55:48, Cong Wang wrote:
> On Mon, Sep 15, 2014 at 2:36 AM, Michal Hocko <mhocko@...e.cz> wrote:
> > This patch builds on top of Cong Wang's. What do you think?
> 
> I think it is should a preliminary of my patch, not a followup.

Well, this looks like a chicken/egg problem. OOM killer currently
doesn't kill any frozen task which is fixed by your patch. So it should
be logically the first to apply. On the other hand that patch opens a
race fixed by this patch. I think the race is really unlikely and we
your patch basically fixes a regression so I would expect it to come
first.

Anyway, I will leave the proper ordering on you if you do not mind.

> I can carry this patch for you if you want.

Would be great, thanks!

> > ---
> > From cdf97a20b107ee584352f07274a88d7c3f014ab2 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@...e.cz>
> > Date: Mon, 15 Sep 2014 10:52:30 +0200
> > Subject: [PATCH] OOM, PM: OOM killed task cannot 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 an window open when a killed task didn't manage to die by the time
> > freeze_processes finishes. Fix this by checking all tasks after OOM
> > killer has been disabled. To prevent from useless check also introduce
> > and check oom_kills count which gets incremented when a task is killed
> > by OOM killer. All the tasks have to be checked only if the counter
> > changes.
> 
> 
> Not sure if I understand your patch correctly, seems you are checking if
> there is any ongoing OOM killer during PM suspend, since oom_kills
> will always increase,

Yeah, I didn't want to pointlessly iterate over all tasks when nothing
happened. OOM is a rare action so this check should be in a slow path.

> maybe a seqlock is more clear?

I wanted to have it as simple as possible and simple counter sounded
like a good fit. seqlock sounds like overkill to me.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/