linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michel Lespinasse <walken@google.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>,
	Arnd Bergmann <arnd@arndb.de>, "Figo. zhang" <figo1802@gmail.com>,
	Aswin Chandramouleeswaran <aswin@hp.com>,
	Rik van Riel <riel@redhat.com>, Waiman Long <waiman.long@hp.com>,
	Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>,
	"Paul E.McKenney" <paulmck@linux.vnet.ibm.com>,
	linux-arch@vger.kernel.org, Andi Kleen <andi@firstfloor.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	George Spelvin <linux@horizon.com>, Ingo Molnar <mingo@elte.hu>,
	Peter Hurley <peter@hurleysoftware.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm <linux-mm@kvack.org>, Alex Shi <alex.shi@linaro.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Scott J Norton <scott.norton@hp.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Dave Hansen <dave.hansen@intel.com>,
	Matthew R Wilcox <matthew.r.wilcox@intel.com>,
	Will Deacon <will.deacon@arm.com>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>
Subject: Re: [PATCH v3 3/5] MCS Lock: Barrier corrections
Date: Thu, 7 Nov 2013 01:55:37 -0800	[thread overview]
Message-ID: <CANN689HkNP-UZOu+vDCFPG5_k=BNZG6a+oP+Ope16vLc2ShFzw@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFyNX=5i0hmk-KuD+Vk+yBD-kkAiywx1Lx_JJmHVPx=1wA@mail.gmail.com>

On Wed, Nov 6, 2013 at 5:39 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Sorry about the HTML crap, the internet connection is too slow for my normal
> email habits, so I'm using my phone.
>
> I think the barriers are still totally wrong for the locking functions.
>
> Adding an smp_rmb after waiting for the lock is pure BS. Writes in the
> locked region could percolate out of the locked region.
>
> The thing is, you cannot do the memory ordering for locks in any same
> generic way. Not using our current barrier system. On x86 (and many others)
> the smp_rmb will work fine, because writes are never moved earlier. But on
> other architectures you really need an acquire to get a lock efficiently. No
> separate barriers. An acquire needs to be on the instruction that does the
> lock.
>
> Same goes for unlock. On x86 any store is a fine unlock, but on other
> architectures you need a store with a release marker.
>
> So no amount of barriers will ever do this correctly. Sure, you can add full
> memory barriers and it will be "correct" but it will be unbearably slow, and
> add totally unnecessary serialization. So *correct* locking will require
> architecture support.

Rather than writing arch-specific locking code, would you agree to
introduce acquire and release memory operations ?

The semantics of an acquire memory operation would be: the specified
memory operation occurs, and any reads or writes after that operation
are guaranteed not to be reordered before it (useful to implement lock
acquisitions).
The semantics of a release memory operation would be: the specified
memory operation occurs, and any reads or writes before that operation
are guaranteed not to be reordered after it (useful to implement lock
releases).

Now each arch would still need to define several acquire and release
operations, but this is a quite useful model to build generic code on.
For example, the fast path for the x86 spinlock implementation could
be expressed generically as an acquire fetch-and-add (for
__ticket_spin_lock) and a release add (for __ticket_spin_unlock).

Would you think this is a useful direction to move to ?

Thanks,

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2013-11-07  9:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1383771175.git.tim.c.chen@linux.intel.com>
2013-11-06 21:36 ` [PATCH v3 0/4] MCS Lock: MCS lock code cleanup and optimizations Tim Chen
2013-11-06 21:41   ` Davidlohr Bueso
2013-11-06 23:55     ` Tim Chen
2013-11-06 21:42   ` H. Peter Anvin
2013-11-06 21:59     ` Michel Lespinasse
2013-11-06 21:37 ` [PATCH v3 1/5] MCS Lock: Restructure the MCS lock defines and locking code into its own file Tim Chen
2013-11-06 21:37 ` [PATCH v3 2/5] MCS Lock: optimizations and extra comments Tim Chen
2013-11-06 21:47   ` Tim Chen
2013-11-06 21:37 ` [PATCH v3 3/5] MCS Lock: Barrier corrections Tim Chen
2013-11-07  1:39   ` Linus Torvalds
2013-11-07  4:29     ` Waiman Long
2013-11-07  8:13     ` Ingo Molnar
2013-11-07  8:22       ` Linus Torvalds
2013-11-07  8:25         ` Ingo Molnar
2013-11-07  9:55     ` Michel Lespinasse [this message]
2013-11-07 12:06       ` Linus Torvalds
2013-11-07 12:50         ` Michel Lespinasse
2013-11-07 14:31           ` Paul E. McKenney
2013-11-07 19:59             ` Michel Lespinasse
2013-11-07 21:15               ` Tim Chen
2013-11-07 22:21                 ` Peter Zijlstra
2013-11-07 22:43                   ` Michel Lespinasse
2013-11-08  1:16                     ` Tim Chen
2013-11-06 21:37 ` [PATCH v3 4/5] MCS Lock: Make mcs_spinlock.h includable in other files Tim Chen
2013-11-06 21:41   ` Tim Chen
2013-11-06 21:37 ` [PATCH v3 5/5] MCS Lock: Allow architecture specific memory barrier in lock/unlock Tim Chen
2013-11-06 21:42   ` Tim Chen

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='CANN689HkNP-UZOu+vDCFPG5_k=BNZG6a+oP+Ope16vLc2ShFzw@mail.gmail.com' \
    --to=walken@google.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.shi@linaro.org \
    --cc=andi@firstfloor.org \
    --cc=arnd@arndb.de \
    --cc=aswin@hp.com \
    --cc=dave.hansen@intel.com \
    --cc=davidlohr.bueso@hp.com \
    --cc=figo1802@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@horizon.com \
    --cc=matthew.r.wilcox@intel.com \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peter@hurleysoftware.com \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=scott.norton@hp.com \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=waiman.long@hp.com \
    --cc=will.deacon@arm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox