Re: [PATCH 5/5] futex: fix miss ordered wakeups

From: Thomas Gleixner
Date: Thu Jun 12 2008 - 11:30:01 EST


On Thu, 12 Jun 2008, Daniel Walker wrote:
> On Thu, 2008-06-12 at 15:33 +0200, Thomas Gleixner wrote:
> > On Thu, 12 Jun 2008, Daniel Walker wrote:
> > > On Thu, 2008-06-12 at 10:56 +0200, Thomas Gleixner wrote:
> > > > Please stop wasting everyone's time with that.
> > >
> > > It achieves correct ordering of the futex waiters inside the kernel,
> > > that is in fact _something_ ..
> >
> > Yeah, just something _useless_
>
> Just because you don't use it, doesn't make it useless .. At least
> there's enough people asking for this that it warrants me writing it..

Which is not really a good technical reason to merge such a
patch. Your handwaving about "enough people" is just irrelevant. Are
you going to implement a root hole as well when enough people ask for
it ?

But there is also a Real Good technical reason why these patches are
going nowhere else than into /dev/null:

your approach of hijacking blocked_on is fundamentaly wrong as it
mixes kernel internal state with user space state.

It will break in preempt-rt at the point when this state is set and
the code blocks on a spinlock, which uses the rtmutex based sleeping
spinlock implementation and overwrites blocked_on.

If it can acquire the spinlock in the fast path without modifying
blocked_on it will cause trouble with the priority boosting chain
when a higher priority task becomes blocked on the spinlock.


If there would be a real good technical reason to fix this priority
ordering it could be done with less than 20 lines of code without
extra locking and wreckage waiting left and right, but I have not yet
seen a single convincing technical argument or a relevant use case
which might justify that.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/