From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
Thomas Gleixner <tglx@linutronix.de>,
Oleg Nesterov <oleg@tv-sign.ru>,
Steven Rostedt <rostedt@goodmis.org>,
Christoph Lameter <clameter@sgi.com>
Subject: Re: [PATCH -rt 4/5] use migrate_disable for __local_begin
Date: Sat, 14 Jul 2007 13:16:44 -0400 [thread overview]
Message-ID: <20070714171644.GC6975@Krystal> (raw)
In-Reply-To: <20070714175840.153584000@chello.nl>
* Peter Zijlstra (a.p.zijlstra@chello.nl) wrote:
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> include/asm-i386/local.h | 7 ++++---
> include/asm-x86_64/local.h | 7 ++++---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/include/asm-i386/local.h
> ===================================================================
> --- linux-2.6.orig/include/asm-i386/local.h
> +++ linux-2.6/include/asm-i386/local.h
> @@ -197,11 +197,12 @@ static __inline__ long local_sub_return(
> #define __local_begin(__flags) \
> { \
> (__flags) = 0; \
> - preempt_disable(); \
> + migrate_disable(); \
Brrrr. That's wrong. Your non atomic __local*() updates only makes sense
when preempt_disable/enable() protects them from concurrent threads on
the same CPU, which is not the case of migrate_disable/enable(). This is
why I suggest that you use local_begin/end() mapped to
migrate_disable/enable() for normal local variables, and, if you really
want a __local_begin/end(), then it should be mapped to
preempt_disable/enable() and should state that it provides no protection
against interrupts.
Mathieu
> }
>
> -static inline void __local_end(unsigned long flags) {
> - preempt_enable();
> +static inline void __local_end(unsigned long flags)
> +{
> + migrate_enable();
> }
>
> /* On x86, these are no better than the atomic variants. */
> Index: linux-2.6/include/asm-x86_64/local.h
> ===================================================================
> --- linux-2.6.orig/include/asm-x86_64/local.h
> +++ linux-2.6/include/asm-x86_64/local.h
> @@ -186,11 +186,12 @@ static __inline__ long local_sub_return(
> #define __local_begin(__flags) \
> { \
> (__flags) = 0; \
> - preempt_disable(); \
> + migrate_disable(); \
> }
>
> -static inline void __local_end(unsigned long flags) {
> - preempt_enable();
> +static inline void __local_end(unsigned long flags)
> +{
> + migrate_enable();
> }
>
> #define __local_inc(l) local_inc(l)
>
> --
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
next prev parent reply other threads:[~2007-07-14 17:16 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-14 17:57 [PATCH -rt 0/5] making SLUB -rt friendly Peter Zijlstra
2007-07-14 17:57 ` [PATCH -rt 1/5] workqueue: queue_work_cpu Peter Zijlstra
2007-07-14 17:14 ` Oleg Nesterov
2007-07-14 18:00 ` Peter Zijlstra
2007-07-14 17:57 ` [PATCH -rt 2/5] Thread Migration Preemption - v2 Peter Zijlstra
2007-07-14 16:49 ` Mathieu Desnoyers
2007-07-14 17:16 ` Oleg Nesterov
2007-07-14 17:34 ` Peter Zijlstra
2007-07-14 18:44 ` Peter Zijlstra
2007-07-14 19:07 ` Peter Zijlstra
2007-07-14 20:39 ` Mathieu Desnoyers
2007-07-14 20:48 ` Oleg Nesterov
2007-07-14 20:53 ` Peter Zijlstra
2007-07-14 17:57 ` [PATCH -rt 3/5] asm/local.h cmpxchg Peter Zijlstra
2007-07-14 16:52 ` Daniel Walker
2007-07-14 17:14 ` Mathieu Desnoyers
2007-07-14 17:31 ` Peter Zijlstra
2007-07-14 18:33 ` Mathieu Desnoyers
2007-07-14 17:57 ` [PATCH -rt 4/5] use migrate_disable for __local_begin Peter Zijlstra
2007-07-14 17:16 ` Mathieu Desnoyers [this message]
2007-07-14 17:32 ` Peter Zijlstra
2007-07-14 18:35 ` Mathieu Desnoyers
2007-07-14 18:41 ` Peter Zijlstra
2007-07-14 18:52 ` Mathieu Desnoyers
2007-07-14 17:57 ` [PATCH -rt 5/5] slub: -rt port Peter Zijlstra
2007-07-14 17:39 ` Oleg Nesterov
2007-07-14 17:50 ` Peter Zijlstra
2007-07-14 19:38 ` Oleg Nesterov
2007-07-14 19:49 ` Peter Zijlstra
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=20070714171644.GC6975@Krystal \
--to=mathieu.desnoyers@polymtl.ca \
--cc=a.p.zijlstra@chello.nl \
--cc=clameter@sgi.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=oleg@tv-sign.ru \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox