All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Nicholas Mc Guire <der.herr@hofr.at>
Cc: linux-rt-users@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Carsten Emde <C.Emde@osadl.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andreas Platschek <platschek@ict.tuwien.ac.at>
Subject: Re: [PATCH RT 5/5] allow preemption in slab_alloc_node and slab_free
Date: Fri, 14 Feb 2014 15:07:29 +0100	[thread overview]
Message-ID: <20140214140729.GI28438@linutronix.de> (raw)
In-Reply-To: <20140210154016.GF20017@opentech.at>

* Nicholas Mc Guire | 2014-02-10 16:40:16 [+0100]:

>__slab_alloc is only called from slub.c:slab_alloc_node
>it runs with local irqs disabled so it can't be pushed off this CPU
>asynchronously, the preempt_disable/enable is thus not needed.
>Aside from that the later this_cpu_cmpxchg_double would catch such a 
>migration event anyay.

Not sure what to do with this one. You do write a longer chapter why it
is okay to drop the preemption disable section and that
this_cpu_cmpxchg_double() would catch it. And I didn't figure out so
far why need to keep preemption disabled while looking at c->tid but not
at c->page.
However, it seems that Christoph Lameter found it important to add a
note in the comment that this preemption disable here is important.
Looking at commit 7cccd80 ("slub: tid must be retrieved from the percpu
area of the current processor") it seems that Steven Rostedt run into
trouble and now we have that preemption_disable() here.

So if you really get better performance and you haven't seen anything
bad happen then you might want to check with Lameter & Rostedt about
your patch and getting it merged upstream.
The commit I mentioned is upstream since v3.11-rc1 and I can see it in
v3.8-RT tree so it looks serious.
I fail to see it in v3.2-RT, Steven, isn't this something we want there,
too?

Sebastian

      reply	other threads:[~2014-02-14 14:07 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-10 15:40 [PATCH RT 5/5] allow preemption in slab_alloc_node and slab_free Nicholas Mc Guire
2014-02-14 14:07 ` Sebastian Andrzej Siewior [this message]

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=20140214140729.GI28438@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=C.Emde@osadl.org \
    --cc=der.herr@hofr.at \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=platschek@ict.tuwien.ac.at \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.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.