linux-kernel - Re: [patch][rfc] x86, mutex: non-atomic unlock (and a rant)

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: <20091102164626.GA10072@lenovo>
Date:	Mon, 2 Nov 2009 19:46:26 +0300
From:	Cyrill Gorcunov <gorcunov@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Nick Piggin <npiggin@...e.de>, Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [patch][rfc] x86, mutex: non-atomic unlock (and a rant)

[Linus Torvalds - Mon, Nov 02, 2009 at 07:20:08AM -0800]
| 
| On Mon, 2 Nov 2009, Nick Piggin wrote:
| > 
| > Non-atomic unlock for mutexs maybe? I do this by relying on cache
| > coherence on a cacheline basis for ordering rather than the memory
| > consistency of the x86. Linus I know you've told me this is an incorrect
| > assumption in the past, but I'm not so sure.
| 
| I'm sure.
| 
| This is simply buggy:
| 
| > +	atomic_set(&lock->count, 1);
| > +	barrier();
| > +	if (unlikely(lock->waiters))
| > +		fail_fn(lock);
| 
| because it doesn't matter one whit whether 'lock->count' and 
| 'lock->waiters' are in the same cacheline or not.
| 
| The cache coherency deals in cachelines, but the instruction re-ordering 
| logic does not. It's entirely possible that the CPU will turn this into
| 
| 	tmp = lock->waiters;
| 	...
| 	atomic_set(&lock->count, 1);
| 	if (tmp)
| 		fail_fn(lock);
| 
| and your "barrier()" did absolutely nothing.
...

If we write it as

	atomic_set(&lock->count, 1);
	some-serializing-op(); /* say cpuid() */
	if (unlikely(lock->waiters))
		fail_fn(lock);

This should do the trick, though this serializing operation
is always cost too much.

The other option could be that we put two mem-write operations
like
	int tmp;
	atomic_set(&lock->count, 1);
	tmp = lock->waiters;
	rmb();
	lock->waiters = tmp;
	if (unlikely(lock->waiters))
		fail_fn(lock);

Which should work faster then cpuid (and we have to be sure somehow
that gcc doesn't suppress this redundant operations).

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