public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>, Andi Kleen <ak@linux.intel.com>,
	Mike Galbraith <bitbucket@online.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Subject: Re: [RFC][PATCH 0/5] preempt_count rework
Date: Wed, 14 Aug 2013 18:04:16 +0200	[thread overview]
Message-ID: <20130814160416.GI24092@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <520B8A81.1080405@zytor.com>

On Wed, Aug 14, 2013 at 06:47:45AM -0700, H. Peter Anvin wrote:

> I still don't see this using a decrement of the percpu variable
> anywhere.  The C compiler doesn't know how to generate those, so if I'm
> not completely wet we will end up relying on sub_preempt_count()...
> which, because it relies on taking the address of the percpu variable
> will generate absolutely horrific code.
> 
> On x86, you never want to take the address of a percpu variable if you
> can avoid it, as you end up generating code like:
> 
> 	movq %fs:0,%rax
> 	subl $1,(%rax)
> 

Urgh,. yes you're right. I keep forgetting GCC doesn't know how to merge
those :/

OK, so something like the below would cure the worst of that I suppose.
It compiles but doesn't boot; must've done something wrong.

Someone please look at it because my asm-foo blows. I pretty much
copy/pasted this from asm/percpu.h.

---
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -20,6 +20,28 @@ static __always_inline int *preempt_coun
 	return &__raw_get_cpu_var(__preempt_count);
 }
 
+#define __preempt_count_add(x) do {		\
+	asm("addl %1," __percpu_arg(0)		\
+		: "+m" (__preempt_count) 	\
+		: "ri" ((int)x) 		\
+		: "memory"); 			\
+} while (0)
+
+#define __preempt_count_sub(x) do {		\
+	asm("subl %1," __percpu_arg(0)		\
+		: "+m" (__preempt_count) 	\
+		: "ri" ((int)x) 		\
+		: "memory"); 			\
+} while (0)
+
+#define preempt_enable() do {			\
+	asm("\nsubl $1," __percpu_arg(0)	\
+	    "\njnz 1f"				\
+	    "\ncall preempt_schedule"		\
+	    "\n1:" : "+m" (__preempt_count)	\
+		   : : "memory");		\
+} while (0)
+
 /*
  * must be macros to avoid header recursion hell
  */
--- a/include/asm-generic/preempt.h
+++ b/include/asm-generic/preempt.h
@@ -17,6 +17,9 @@ static __always_inline int *preempt_coun
 	return &current_thread_info()->preempt_count;
 }
 
+#define __preempt_count_add(x)	do { current_thread_info()->preempt_count += (x); } while (0)
+#define __preempt_count_sub(x)  __preempt_count_add(-(x))
+
 /*
  * must be macros to avoid header recursion hell
  */
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -45,8 +45,8 @@ static __always_inline bool test_preempt
   extern void add_preempt_count(int val);
   extern void sub_preempt_count(int val);
 #else
-# define add_preempt_count(val)	do { *preempt_count_ptr() += (val); } while (0)
-# define sub_preempt_count(val)	do { *preempt_count_ptr() -= (val); } while (0)
+# define add_preempt_count(val)	__preempt_count_add(val)
+# define sub_preempt_count(val)	__preempt_count_sub(val)
 #endif
 
 #define inc_preempt_count() add_preempt_count(1)
@@ -101,17 +101,17 @@ do { \
 
 #define preempt_enable_no_resched()	sched_preempt_enable_no_resched()
 
+#ifndef preempt_enable
 #define preempt_enable() \
 do { \
 	preempt_enable_no_resched(); \
 	preempt_check_resched(); \
 } while (0)
+#endif
 
 /* For debugging and tracer internals only! */
-#define add_preempt_count_notrace(val)			\
-	do { *preempt_count_ptr() += (val); } while (0)
-#define sub_preempt_count_notrace(val)			\
-	do { *preempt_count_ptr() -= (val); } while (0)
+#define add_preempt_count_notrace(val)	__preempt_count_add(val)
+#define sub_preempt_count_notrace(val)	__preempt_count_sub(val)
 #define inc_preempt_count_notrace() add_preempt_count_notrace(1)
 #define dec_preempt_count_notrace() sub_preempt_count_notrace(1)
 

  parent reply	other threads:[~2013-08-14 16:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-14 13:15 [RFC][PATCH 0/5] preempt_count rework Peter Zijlstra
2013-08-14 13:15 ` [RFC][PATCH 1/5] sched: Introduce preempt_count accessor functions Peter Zijlstra
2013-08-14 13:15 ` [RFC][PATCH 2/5] sched: Add NEED_RESCHED to the preempt_count Peter Zijlstra
2013-08-14 13:15 ` [RFC][PATCH 3/5] sched, arch: Create asm/preempt.h Peter Zijlstra
2013-08-14 13:15 ` [RFC][PATCH 4/5] sched: Create more preempt_count accessors Peter Zijlstra
2013-08-14 13:15 ` [RFC][PATCH 5/5] sched, x86: Provide a per-cpu preempt_count implementation Peter Zijlstra
2013-08-14 13:47 ` [RFC][PATCH 0/5] preempt_count rework H. Peter Anvin
2013-08-14 15:39   ` Mike Galbraith
2013-08-14 15:43     ` H. Peter Anvin
2013-08-15  9:01       ` Mike Galbraith
2013-08-14 16:06     ` Peter Zijlstra
2013-08-14 16:14       ` H. Peter Anvin
2013-08-14 16:52         ` Peter Zijlstra
2013-08-14 16:58           ` H. Peter Anvin
2013-08-14 16:04   ` Peter Zijlstra [this message]
2013-08-14 17:31     ` Peter Zijlstra
2013-08-14 16:48 ` Andi Kleen
2013-08-14 16:55   ` Peter Zijlstra
2013-08-14 17:12     ` Andi Kleen

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=20130814160416.GI24092@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ak@linux.intel.com \
    --cc=arjan@linux.intel.com \
    --cc=bitbucket@online.de \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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