LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Arkadiusz Miskiewicz <arekm@maven.pl>, linux-kernel@vger.kernel.org
Subject: Re: proxy_pda was Re: What was in the x86 merge for .20
Date: Fri, 8 Dec 2006 22:22:33 +0100	[thread overview]
Message-ID: <200612082222.33673.ak@suse.de> (raw)
In-Reply-To: <4579D496.6080201@goop.org>

On Friday 08 December 2006 22:09, Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
> > Looking at Arkadiusz' output file it looks like gcc 4.2 decided to CSE the
> > address :/
> >
> > 	movl	$_proxy_pda+8, %edx	#, tmp65
> >
> > Very sad, but legitimate.
> >   
> 
> Yes, that was my conclusion too.  Though in this case the code could be
> cleaned up by cutting down on the number of uses of "current" - but
> that's hardly a general fix.
> 
> > The only workaround I can think of would be to define it as a symbol
> > (or away in vmlinux.lds.S)
> 
> Yes.  I was thinking about setting it in vmlinux.lds to some obviously
> bad address so that any access will be highly visible.
> 
> > . Or do away with the idea of proxy_pda
> > again.
> >   
> 
> That would be very sad indeed.

The trouble is when it's CSEd it actually causes worse code because
a register is tied up. That might not be worth the advantage of having it?

Hmm, maybe marking it volatile would help? Arkadiusz, does the following patch
help?

-Andi

Work around gcc 4.2 CSEing the proxy_pda

proxy_pda doesn't exactly exist -- we just use it to describe side effects
of the PDA manipulation to gcc. But when gcc 4.2 CSEs the address
it generates references and worse code and the link breaks.

Let's cast it to volatile and see if it that helps.

TBD same for x86-64
Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/include/asm-i386/pda.h
===================================================================
--- linux.orig/include/asm-i386/pda.h
+++ linux/include/asm-i386/pda.h
@@ -40,19 +40,19 @@ extern struct i386_pda _proxy_pda;
 		switch (sizeof(_proxy_pda.field)) {			\
 		case 1:							\
 			asm(op "b %1,%%gs:%c2"				\
-			    : "+m" (_proxy_pda.field)			\
+			    : "+m" (*(volatile T__ *)&_proxy_pda.field)			\
 			    :"ri" ((T__)val),				\
 			     "i"(pda_offset(field)));			\
 			break;						\
 		case 2:							\
 			asm(op "w %1,%%gs:%c2"				\
-			    : "+m" (_proxy_pda.field)			\
+			    : "+m" (*(volatile T__ *)&_proxy_pda.field)			\
 			    :"ri" ((T__)val),				\
 			     "i"(pda_offset(field)));			\
 			break;						\
 		case 4:							\
 			asm(op "l %1,%%gs:%c2"				\
-			    : "+m" (_proxy_pda.field)			\
+			    : "+m" (*(volatile T__ *)&_proxy_pda.field)			\
 			    :"ri" ((T__)val),				\
 			     "i"(pda_offset(field)));			\
 			break;						\
@@ -63,24 +63,25 @@ extern struct i386_pda _proxy_pda;
 #define pda_from_op(op,field)						\
 	({								\
 		typeof(_proxy_pda.field) ret__;				\
+		typedef typeof(_proxy_pda.field) T__;			\
 		switch (sizeof(_proxy_pda.field)) {			\
 		case 1:							\
 			asm(op "b %%gs:%c1,%0"				\
 			    : "=r" (ret__)				\
 			    : "i" (pda_offset(field)),			\
-			      "m" (_proxy_pda.field));			\
+			      "m" (*(volatile T__ *)&_proxy_pda.field));			\
 			break;						\
 		case 2:							\
 			asm(op "w %%gs:%c1,%0"				\
 			    : "=r" (ret__)				\
 			    : "i" (pda_offset(field)),			\
-			      "m" (_proxy_pda.field));			\
+			      "m" (*(volatile T__ *)&_proxy_pda.field));			\
 			break;						\
 		case 4:							\
 			asm(op "l %%gs:%c1,%0"				\
 			    : "=r" (ret__)				\
 			    : "i" (pda_offset(field)),			\
-			      "m" (_proxy_pda.field));			\
+			      "m" (*(volatile T__ *)&_proxy_pda.field));			\
 			break;						\
 		default: __bad_pda_field();				\
 		}							\

  reply	other threads:[~2006-12-08 21:23 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-08  3:01 Andi Kleen
2006-12-08 10:08 ` Andrew Morton
2006-12-08 10:44   ` Ingo Molnar
2006-12-08 16:41   ` Siddha, Suresh B
2006-12-08 17:10     ` Andi Kleen
2006-12-08 18:00       ` Siddha, Suresh B
2006-12-08 20:15         ` [discuss] " Andi Kleen
2006-12-09  8:41         ` Andrew Morton
2006-12-09 18:51           ` Andi Kleen
2006-12-08 17:12   ` Andi Kleen
2006-12-08 12:04 ` Arkadiusz Miskiewicz
2006-12-08 12:51   ` Muli Ben-Yehuda
2006-12-08 13:03     ` Arkadiusz Miskiewicz
2006-12-08 16:34       ` Muli Ben-Yehuda
2006-12-08 18:04       ` Andi Kleen
2006-12-08 18:10         ` Arkadiusz Miskiewicz
2006-12-08 20:07           ` Andi Kleen
2006-12-08 20:36             ` Jeremy Fitzhardinge
2006-12-08 20:35   ` Jeremy Fitzhardinge
2006-12-08 21:06     ` proxy_pda was " Andi Kleen
2006-12-08 21:09       ` Jeremy Fitzhardinge
2006-12-08 21:22         ` Andi Kleen [this message]
2006-12-08 21:34           ` Jeremy Fitzhardinge
2006-12-08 22:04           ` Arkadiusz Miskiewicz
2007-01-15 20:41 Paweł Sikora

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=200612082222.33673.ak@suse.de \
    --to=ak@suse.de \
    --cc=arekm@maven.pl \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: proxy_pda was Re: What was in the x86 merge for .20' \
    /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

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).