linux-kernel - Re: [PATCH 00/11] nested sleeps, fixes and debug infrastructure

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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-ID: <20141028082335.GM3337@twins.programming.kicks-ass.net>
Date:	Tue, 28 Oct 2014 09:23:35 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Mike Galbraith <umgwanakikbuti@...il.com>, mingo@...nel.org,
	torvalds@...ux-foundation.org, tglx@...utronix.de,
	ilya.dryomov@...tank.com, linux-kernel@...r.kernel.org,
	Eric Paris <eparis@...hat.com>, rafael.j.wysocki@...el.com
Subject: Re: [PATCH 00/11] nested sleeps, fixes and debug infrastructure

On Tue, Oct 28, 2014 at 01:07:03AM +0100, Oleg Nesterov wrote:
> On 10/27, Peter Zijlstra wrote:
> >
> > On Thu, Oct 02, 2014 at 02:15:53PM +0200, Peter Zijlstra wrote:
> > > On Thu, Oct 02, 2014 at 12:22:51PM +0200, Peter Zijlstra wrote:
> >
> > > > +#define __wait_event_freezable(wq, condition)				\
> > > > +	(void)___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0,	\
> > > > +			    schedule(); try_to_freeze())
>                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> see below.

> I was going to say that wait_event_freezable() in kauditd_thread()
> is not friendly wrt kthread_should_stop() and thus we we need
> kthread_freezable_should_stop().

I'm not sure those two would interact, yes, both would first set either
the freezable or stop bit and then wake. If both were to race, all we
need to ensure is to check both before calling schedule again.

A loop like:

	while (!kthread_should_stop()) {
		wait_event_freezable(wq, cond);
	}

Would satisfy that, because even if kthread_should_stop() gets set first
and then freezing happens and we get into try_to_freeze() first, we'd
still to the kthread_should_stop() check right after we thaw.

So I don't se a reason to collect both stop and freeze into a single
thing.

> But in fact we never stop this kauditd_task, so I think we should
> turn the main loop into "for (;;)" and change this code to use
> wait_event_freezable() like your patch does.

Right.

> Perhaps it also makes sense to redefine wait_event_freezable.*()
> via ___wait_event(cmd => freezable_schedule), but I think this needs
> another patch.

So I talked to Rafael yesterday and I'm going to replace all the
wait_event*() stuff, and I suppose also freezable_schedule() because
they're racy.

The moment we call freezer_do_not_count() the freezer will ignore us,
this means the thread could still be running (albeit not for long) when
the freezer reports success.

Ideally I'll be able to kill the entire freezer_do_not_count() stuff.
--
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/