linux-kernel - Re: futex: clarification needed with drop_futex_key

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] [day] [month] [year] [list]

Message-ID: <20160402063919.GD5329@linux-uzut.site>
Date:	Fri, 1 Apr 2016 23:39:19 -0700
From:	Davidlohr Bueso <dave@...olabs.net>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Mike Galbraith <umgwanakikbuti@...il.com>,
	Petros Koutoupis <petros@...roskoutoupis.com>,
	linux-kernel@...r.kernel.org,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	catalin.marinas@....com, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: futex: clarification needed with drop_futex_key_refs and memory
 barriers

On Tue, 29 Mar 2016, Peter Zijlstra wrote:

>In any case; the below (completely irrelevant patch for you) is
>something I would propose. It gives hb_waiter_dec() RELEASE like
>semantics and ensures it cannot creep into the lock sections its
>typically behind. Although strictly speaking I think it being inside
>that lock region is sufficient.

Indeed, it should be sufficient. Racing with waiter decrements
(leaking into the hb critical region) is perfectly ok as the
consequence is that the reader thread will simply go take the lock
by not seeing the 1->0 dec, and therefore no harm done. Something that
0->1 cannot afford to rely on, obviously. So I think we can save the extra
barrier for the release semantics and keep the call relaxed, as the
performance penalty would be higher. Or are you referring to something else?

>
>It also re-orders the increment in requeue to happen before we add to
>the list (as is the proper order)

ack to this part -- should be a separate patch.

Thanks,
Davidlohr