linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	<linux-parisc@vger.kernel>,
	Andrew Morton <akpm@linux-foundation.org>,
	Helge Deller <deller@gmx.de>, <linux-kernel@vger.kernel.org>,
	<stable@vger.kernel.org>,
	"James E.J. Bottomley" <jejb@parisc-linux.org>,
	Pekka Enberg <penberg@kernel.org>, <linux-mm@kvack.org>,
	Noam Camus <noamc@ezchip.com>,
	David Rientjes <rientjes@google.com>,
	Christoph Lameter <cl@linux.com>,
	<linux-snps-arc@lists.infradead.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH] mm: slub: Ensure that slab_unlock() is atomic
Date: Wed, 9 Mar 2016 18:52:45 +0530	[thread overview]
Message-ID: <56E023A5.2000105@synopsys.com> (raw)
In-Reply-To: <20160309101349.GJ6344@twins.programming.kicks-ass.net>

On Wednesday 09 March 2016 03:43 PM, Peter Zijlstra wrote:
>> There is clearly a problem in slub code that it is pairing a test_and_set_bit()
>> with a __clear_bit(). Latter can obviously clobber former if they are not a single
>> instruction each unlike x86 or they use llock/scond kind of instructions where the
>> interim store from other core is detected and causes a retry of whole llock/scond
>> sequence.
> 
> Yes, test_and_set_bit() + __clear_bit() is broken.

But in SLUB: bit_spin_lock() + __bit_spin_unlock() is acceptable ? How so
(ignoring the performance thing for discussion sake, which is a side effect of
this implementation).

So despite the comment below in bit_spinlock.h I don't quite comprehend how this
is allowable. And if say, by deduction, this is fine for LLSC or lock prefixed
cases, then isn't this true in general for lot more cases in kernel, i.e. pairing
atomic lock with non-atomic unlock ? I'm missing something !

| /*
|  *  bit-based spin_unlock()
|  *  non-atomic version, which can be used eg. if the bit lock itself is
|  *  protecting the rest of the flags in the word.
|  */
| static inline void __bit_spin_unlock(int bitnum, unsigned long *addr)

  parent reply	other threads:[~2016-03-09 13:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-08 14:30 [PATCH] mm: slub: Ensure that slab_unlock() is atomic Vineet Gupta
2016-03-08 15:00 ` Christoph Lameter
2016-03-08 15:46   ` Vineet Gupta
2016-03-08 20:40     ` Christoph Lameter
2016-03-09  6:43       ` Vineet Gupta
2016-03-09 10:13         ` Peter Zijlstra
2016-03-09 10:31           ` Peter Zijlstra
2016-03-09 11:12             ` Vineet Gupta
2016-03-09 11:00           ` Vineet Gupta
2016-03-09 11:40             ` Peter Zijlstra
2016-03-09 11:53               ` Vineet Gupta
2016-03-09 12:22                 ` Peter Zijlstra
2016-03-14  8:05               ` Vineet Gupta
2016-03-21 11:16               ` [tip:locking/urgent] bitops: Do not default to __clear_bit() for __clear_bit_unlock() tip-bot for Peter Zijlstra
2016-03-09 13:22           ` Vineet Gupta [this message]
2016-03-09 14:51             ` [PATCH] mm: slub: Ensure that slab_unlock() is atomic Peter Zijlstra
2016-03-10  5:51               ` Vineet Gupta
2016-03-10  9:10                 ` Peter Zijlstra
2016-03-08 15:32 ` Vlastimil Babka

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=56E023A5.2000105@synopsys.com \
    --to=vineet.gupta1@synopsys.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=deller@gmx.de \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jejb@parisc-linux.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-parisc@vger.kernel \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=noamc@ezchip.com \
    --cc=penberg@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=stable@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).