linux-kernel - Re: [PATCH v3 4/4] futex: Avoid taking hb lock if nothing to wakeup

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: <CA+55aFyay84UKWEbAZ=8bT-KV5gq-+1fWgS52gRQv=qZJzMSOQ@mail.gmail.com>
Date:	Thu, 19 Dec 2013 15:53:59 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Davidlohr Bueso <davidlohr@...com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Mike Galbraith <efault@....de>, Jeff Mahoney <jeffm@...e.com>,
	Jason Low <jason.low2@...com>,
	Waiman Long <Waiman.Long@...com>, Tom Vaden <tom.vaden@...com>,
	"Norton, Scott J" <scott.norton@...com>,
	"Chandramouleeswaran, Aswin" <aswin@...com>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH v3 4/4] futex: Avoid taking hb lock if nothing to wakeup

On Thu, Dec 19, 2013 at 3:42 PM, Davidlohr Bueso <davidlohr@...com> wrote:
>
>> And it can look at
>> the wait-list for that - but you want to close the race between the
>> entry actually getting added to the list using this counter. But the
>> place you increment the new counter is the same place as you take the
>> spinlock, which does that ticket increment. No?
>
> I don't think so. If we rely on this, then we could end up missing
> to-be-queued tasks that are in the process of acquiring the lock, so
> waiters could sleep forever. So we need a way of acknowledging that a
> task is in the process of waiting when concurrently doing wakeups.

I don't understand. Why is that any better with the separate atomic count?

The only place you increment the count - hb_waiters_inc() - is called
in exactly two places:

 - in requeue_futex(), which already holds the spinlock on both queues

 - in queue_lock(), immediately before getting the spinlock (which
will do the SAME ATOMIC INCREMENT, except it's just doing it on a
different member of the structure, namely the spinlock head)

so if you drop the waiters increment, and instead use the spinlock
head increment as the "waiters", you get exactly the same semantics.
If anything, for the requeue_futex() case, the spinlock increment was
done *earlier*, but that's really not relevant.

So how could we miss this? Explain to me what the separate counter
does that isn't done by the spinlock head counter.

              Linus
--
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/