From: Linus Torvalds <torvalds@linux-foundation.org>
To: Nick Piggin <npiggin@suse.de>
Cc: Ingo Molnar <mingo@elte.hu>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [patch][rfc] x86, mutex: non-atomic unlock (and a rant)
Date: Mon, 2 Nov 2009 07:20:08 -0800 (PST) [thread overview]
Message-ID: <alpine.LFD.2.01.0911020710030.31845@localhost.localdomain> (raw)
In-Reply-To: <20091102120739.GA20318@wotan.suse.de>
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.
The fact that it may _work_ in almost all circumstances (and perhaps even
"always" on some microarchitectures) is irrelevant. It's simply not
guaranteed to work. Yes, you need just the right timings, and yes, it's
probably hard to hit. And yes, I can well imagine that some micro-
architecture will even guarantee the write->read ordering, and that it
would _always_ work on that micro-architecture.
But I can see your thing failing even on an in-order CPU. It literally
doesn't even need OoO to fail, all it needs is a sufficiently deep write
buffer on an in-order core. And to fail in practice, maybe there needs to
be lots of writes in that buffer, and some bad luck, but the thing is,
write buffers are not coherent between cores - so the write may have
happened as far as the core that does it is concerned, but other cores
(or even HT) may not see the new value until after the read has taken
effect.
Linus
next prev parent reply other threads:[~2009-11-02 15:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-02 12:07 [patch][rfc] x86, mutex: non-atomic unlock (and a rant) Nick Piggin
2009-11-02 15:20 ` Linus Torvalds [this message]
2009-11-02 16:00 ` Nick Piggin
2009-11-02 16:46 ` Cyrill Gorcunov
2009-11-02 18:09 ` Cyrill Gorcunov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LFD.2.01.0911020710030.31845@localhost.localdomain \
--to=torvalds@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=npiggin@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.