linux-kernel - Re: Linux 3.19-rc3

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: <20150106115645.GA26845@kmo-pixel>
Date:	Tue, 6 Jan 2015 03:56:45 -0800
From:	Kent Overstreet <kmo@...erainc.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Sedat Dilek <sedat.dilek@...il.com>,
	Dave Jones <davej@...emonkey.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, Chris Mason <clm@...com>
Subject: Re: Linux 3.19-rc3

On Tue, Jan 06, 2015 at 12:42:15PM +0100, Peter Zijlstra wrote:
> On Tue, Jan 06, 2015 at 03:07:30AM -0800, Kent Overstreet wrote:
> > > No, the root cause is nesting sleep primitives, this is not fixable in
> > > the one place, both prepare_to_wait and mutex_lock are using
> > > task_struct::state, they have to, no way around it.
> > 
> > No, it's completely possible to construct a prepare_to_wait() that doesn't
> > require messing with the task state. Had it for years.
> > 
> > http://evilpiepirate.org/git/linux-bcache.git/log/?h=aio_ring_fix
> 
> Your closures are cute but not the same. And sure you can do a wait
> queue like interface -- my wait_woken thing is an example -- that
> doesn't require task state.
> 
> The point remains that you then have to fix every instance to conform to
> the new interface.

Possibly I missed your point because I've been overly crotchety lately.

I do want to make the point that it's not really the callers that are broken;
especially those that are using prepare_to_wait() via wait_event(). Using
wait_event() is exactly what people _should_ be doing, instead of open coding
this stuff and/or coming up with hacks to work around the fact that
prepare_to_wait() is implemented via messing with the task state.

This is a sore point for me because I've seen other experienced, _skilled_
programmers screw this kind of code up too many times and I hate debugging lost
wakeups. I can write this kind of code in my sleep because I've spent too much
of time time implementing these kinds of primitives and I'm sure you can too,
but most people can't.

Anyways, my point is either wait_event() should be fixed to not muck with the
task state, or since that's not really practical we should at least provide a
standard drop in replacement that doesn't.

And the drop in replacement more or less exists, closure_wait_event() has the
same semantics as wait_event, similarly with the lower level primitives I just
listed the conversions for.

I don't even really care if it's closures, we could make something new that uses
the existing wait_queue_head_t but IMO it's gonna look a lot like closures with
an embedded refcount and closure_sync() instead of schedule().
--
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/