Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-22 Thread H. Peter Anvin
Andi Kleen wrote:
> On Sun, Jul 22, 2007 at 11:05:24AM -0700, H. Peter Anvin wrote:
>> Andi Kleen wrote:
 The main reason is that everyone seems to invoke it either incorrectly
>>> Where is it incorrect? 
>> Using "r" or "m" (as opposed as "+m") gives gcc the wrong dependency
>> information, and it could, at least in theory, cause memory references
>> to be moved around it -- especially when using "r".
> 
> That doesn't sound correct. Taking the address should be like an escaping
> pointer and equivalent to read/write and effectively disable optimizations
> on this memory.

It might be in the existing gcc for all I know, but there is no explicit
guarantee to that effect.  Remember that inline assembly is really
nothing but the guts of gcc exposed to the programmer, and that the gcc
people are very blunt about not wanting to muck with the former to suit
the latter.  This means that there are a lot of things that may
accidentally work in one version of gcc which breaks in another.

There are two documented ways to tell gcc that you're mucking with
memory, and those are "m" constraints ("m" read, "=m" write, "+m"
read/write) and "memory" clobbers; the latter which affects all memory.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-22 Thread Andi Kleen
On Sun, Jul 22, 2007 at 11:05:24AM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >> The main reason is that everyone seems to invoke it either incorrectly
> > Where is it incorrect? 
> 
> Using "r" or "m" (as opposed as "+m") gives gcc the wrong dependency
> information, and it could, at least in theory, cause memory references
> to be moved around it -- especially when using "r".

That doesn't sound correct. Taking the address should be like an escaping
pointer and equivalent to read/write and effectively disable optimizations
on this memory.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-22 Thread H. Peter Anvin
Andi Kleen wrote:
>> The main reason is that everyone seems to invoke it either incorrectly
> Where is it incorrect? 

Using "r" or "m" (as opposed as "+m") gives gcc the wrong dependency
information, and it could, at least in theory, cause memory references
to be moved around it -- especially when using "r".

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-22 Thread Andi Kleen
On Fri, Jul 20, 2007 at 02:33:46PM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> > On Fri, Jul 20, 2007 at 02:19:58PM -0700, H. Peter Anvin wrote:
> >> Create an inline function for clflush(), with the proper arguments,
> >> and use it instead of hard-coding the instruction.
> >>
> >> This also removes one instance of hard-coded wbinvd, based on a patch
> >> by Bauder de Oliveira Costa.
> > 
> > I don't see much sense in it. CLFLUSH is not priviledged, paravirt
> > doesn't need to change and this adds just an unnecessary layer of 
> > abstraction.
> 
> The main reason is that everyone seems to invoke it either incorrectly

Where is it incorrect? 

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread Muli Ben-Yehuda
On Sat, Jul 21, 2007 at 03:09:41PM -0700, H. Peter Anvin wrote:
> Muli Ben-Yehuda wrote:
> > 
> > Ok, let's try again:
> > 
> > You're changing this (pageattr.c)
> > 
> > asm volatile("clflush (%0)" :: "r" (adr + i));
> > 
> > into this:
> > 
> > asm volatile("clflush %0" : "+m" (*(char __force*)(adr + i)));
> > 
> > The original one calls clflush with (adr + i), the new one with (*(adr
> > + i)). Are these calls equivalent?
> 
> Yes, they are.  The parentheses which are part of the old assembly
> string has the same effect as the asterisk operator in C.

Ah, I see. Thanks for the explanation!

Cheers,
Muli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread H. Peter Anvin
H. Peter Anvin wrote:
> Muli Ben-Yehuda wrote:
>> Ok, let's try again:
>>
>> You're changing this (pageattr.c)
>>
>> asm volatile("clflush (%0)" :: "r" (adr + i));
>>
>> into this:
>>
>>  asm volatile("clflush %0" : "+m" (*(char __force*)(adr + i)));
>>
>> The original one calls clflush with (adr + i), the new one with (*(adr
>> + i)). Are these calls equivalent?
> 
> Yes, they are.  The parentheses which are part of the old assembly
> string has the same effect as the asterisk operator in C.
> 
> The difference between the two is that the latter form allows the C
> compiler to select the addressing mode, which allows the full range of
> addressing modes, whereas the former forces it to use a single register
> indirect.
> 

Just to be absolutely obvious about it:

: tazenda 15 ; cat demo.c
#define __force

static inline void clflush1(volatile void *__p)
{
asm volatile("clflush %0" : "+m" (*(char __force *)__p));
}

static inline void clflush2(volatile void *__p)
{
asm volatile("clflush (%0)" :: "r" (__p));
}

void demo(void *q)
{
clflush1(q);
clflush2(q);
}
: tazenda 16 ; gcc -m32 -O3 -S demo.c
: tazenda 17 ; cat demo.s
.file   "demo.c"
.text
.p2align 4,,15
.globl demo
.type   demo, @function
demo:
pushl   %ebp
movl%esp, %ebp
movl8(%ebp), %eax
#APP
clflush (%eax)
clflush (%eax)
#NO_APP
popl%ebp
ret
.size   demo, .-demo
.ident  "GCC: (GNU) 4.1.2 20070626 (Red Hat 4.1.2-13)"
.section.note.GNU-stack,"",@progbits
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread H. Peter Anvin
Muli Ben-Yehuda wrote:
> 
> Ok, let's try again:
> 
> You're changing this (pageattr.c)
> 
> asm volatile("clflush (%0)" :: "r" (adr + i));
> 
> into this:
> 
>   asm volatile("clflush %0" : "+m" (*(char __force*)(adr + i)));
> 
> The original one calls clflush with (adr + i), the new one with (*(adr
> + i)). Are these calls equivalent?

Yes, they are.  The parentheses which are part of the old assembly
string has the same effect as the asterisk operator in C.

The difference between the two is that the latter form allows the C
compiler to select the addressing mode, which allows the full range of
addressing modes, whereas the former forces it to use a single register
indirect.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread Muli Ben-Yehuda
On Sat, Jul 21, 2007 at 01:18:54PM -0700, H. Peter Anvin wrote:
> Muli Ben-Yehuda wrote:
> > 
> > So it looks like we have a purely syntactic patch that does something
> > different than the original code in one of the above places. What am I
> > missing?
> > 
> 
> +static inline void clflush(volatile void *__p)
> +{
> + asm volatile("clflush %0" : "+m" (*(char __force *)__p));
>   ^
> +}

Ok, let's try again:

You're changing this (pageattr.c)

asm volatile("clflush (%0)" :: "r" (adr + i));

into this:

asm volatile("clflush %0" : "+m" (*(char __force*)(adr + i)));

The original one calls clflush with (adr + i), the new one with (*(adr
+ i)). Are these calls equivalent? if not, and I don't think they are,
you are changing the semantics of the code (presumably, because it
fixes a bug), and *that should be a separate patch*.

Cheers,
Muli

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread H. Peter Anvin
Muli Ben-Yehuda wrote:
> 
> So it looks like we have a purely syntactic patch that does something
> different than the original code in one of the above places. What am I
> missing?
> 

+static inline void clflush(volatile void *__p)
+{
+   asm volatile("clflush %0" : "+m" (*(char __force *)__p));
  ^
+}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread Muli Ben-Yehuda
On Sat, Jul 21, 2007 at 12:52:26PM -0700, H. Peter Anvin wrote:
> Muli Ben-Yehuda wrote:
> >
> > Mostly looks good, but see below.
> >
> >> diff --git a/drivers/char/agp/efficeon-agp.c 
> >> b/drivers/char/agp/efficeon-agp.c
> >> index df8da72..41f46df 100644
> >> --- a/drivers/char/agp/efficeon-agp.c
> >> +++ b/drivers/char/agp/efficeon-agp.c
> >> @@ -221,7 +221,7 @@ static int efficeon_create_gatt_table(struct 
> >> agp_bridge_data *bridge)
> >>SetPageReserved(virt_to_page((char *)page));
> >>
> >>for (offset = 0; offset < PAGE_SIZE; offset += clflush_chunk)
> >> -  asm volatile("clflush %0" : : "m" (*(char 
> >> *)(page+offset)));
> >> +  clflush((char *)page+offset);
> >
> > The original code flushes (*(page+offset)) whereas the new code
> > flushes (pagee+offset). Assuming this is a bug-fix, it should go as a
> > separate patch.
>
> No, it doesn't.  There is a * in the inline, as there should be.

I guess I don't follow. Compare:

diff --git a/arch/i386/mm/pageattr.c b/arch/i386/mm/pageattr.c
index 37992ff..cb4ded4 100644
--- a/arch/i386/mm/pageattr.c
+++ b/arch/i386/mm/pageattr.c
@@ -70,10 +70,10 @@ static struct page *split_large_page(unsigned long
address, pgprot_t prot,

 static void cache_flush_page(struct page *p)
 {
-   unsigned long adr = (unsigned long)page_address(p);
+   void *adr = page_address(p);
int i;
for (i = 0; i < PAGE_SIZE; i +=
boot_cpu_data.x86_clflush_size)
-   asm volatile("clflush (%0)" :: "r" (adr + i));
+   clflush(adr+i);
 }

Original code: clflush (adr + i).
New code: clflush (adr + i)

diff --git a/drivers/char/agp/efficeon-agp.c b/drivers/char/agp/efficeon-agp.c
index df8da72..41f46df 100644
--- a/drivers/char/agp/efficeon-agp.c
+++ b/drivers/char/agp/efficeon-agp.c
@@ -221,7 +221,7 @@ static int efficeon_create_gatt_table(struct 
agp_bridge_data *bridge)
SetPageReserved(virt_to_page((char *)page));

for (offset = 0; offset < PAGE_SIZE; offset += clflush_chunk)
-   asm volatile("clflush %0" : : "m" (*(char 
*)(page+offset)));
+   clflush((char *)page+offset);

Original code: clflush *(page + offset)
   ^
New code: clflush (page + offset)

So it looks like we have a purely syntactic patch that does something
different than the original code in one of the above places. What am I
missing?

Cheers,
Muli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread H. Peter Anvin
Muli Ben-Yehuda wrote:
> 
> Mostly looks good, but see below.
> 
>> diff --git a/drivers/char/agp/efficeon-agp.c 
>> b/drivers/char/agp/efficeon-agp.c
>> index df8da72..41f46df 100644
>> --- a/drivers/char/agp/efficeon-agp.c
>> +++ b/drivers/char/agp/efficeon-agp.c
>> @@ -221,7 +221,7 @@ static int efficeon_create_gatt_table(struct 
>> agp_bridge_data *bridge)
>>  SetPageReserved(virt_to_page((char *)page));
>>  
>>  for (offset = 0; offset < PAGE_SIZE; offset += clflush_chunk)
>> -asm volatile("clflush %0" : : "m" (*(char 
>> *)(page+offset)));
>> +clflush((char *)page+offset);
> 
> The original code flushes (*(page+offset)) whereas the new code
> flushes (pagee+offset). Assuming this is a bug-fix, it should go as a
> separate patch.
> 

No, it doesn't.  There is a * in the inline, as there should be.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-21 Thread Muli Ben-Yehuda
On Fri, Jul 20, 2007 at 02:19:58PM -0700, H. Peter Anvin wrote:
> Create an inline function for clflush(), with the proper arguments,
> and use it instead of hard-coding the instruction.
> 
> This also removes one instance of hard-coded wbinvd, based on a patch
> by Bauder de Oliveira Costa.
> 
> Cc: Andi Kleen <[EMAIL PROTECTED]>
> Cc: Glauber de Oliveira Costa <[EMAIL PROTECTED]>
> Signed-off-by: H. Peter Anvin <[EMAIL PROTECTED]>

Mostly looks good, but see below.

> diff --git a/drivers/char/agp/efficeon-agp.c b/drivers/char/agp/efficeon-agp.c
> index df8da72..41f46df 100644
> --- a/drivers/char/agp/efficeon-agp.c
> +++ b/drivers/char/agp/efficeon-agp.c
> @@ -221,7 +221,7 @@ static int efficeon_create_gatt_table(struct 
> agp_bridge_data *bridge)
>   SetPageReserved(virt_to_page((char *)page));
>  
>   for (offset = 0; offset < PAGE_SIZE; offset += clflush_chunk)
> - asm volatile("clflush %0" : : "m" (*(char 
> *)(page+offset)));
> + clflush((char *)page+offset);

The original code flushes (*(page+offset)) whereas the new code
flushes (pagee+offset). Assuming this is a bug-fix, it should go as a
separate patch.

> @@ -268,15 +268,16 @@ static int efficeon_insert_memory(struct agp_memory * 
> mem, off_t pg_start, int t
>   *page = insert;
>  
>   /* clflush is slow, so don't clflush until we have to */
> - if ( last_page &&
> -  ((unsigned long)page^(unsigned long)last_page) & 
> clflush_mask )
> - asm volatile("clflush %0" : : "m" (*last_page));
> + if (last_page &&
> + (((unsigned long)page^(unsigned long)last_page) &
> +  clflush_mask))
> + clflush(last_page);

Same thing.


>   last_page = page;
>   }
>  
>   if ( last_page )
> - asm volatile("clflush %0" : : "m" (*last_page));
> + clflush(last_page);

Here too.

Cheers,
Muli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-20 Thread H. Peter Anvin
Glauber de Oliveira Costa wrote:
> On Fri, 2007-07-20 at 14:19 -0700, H. Peter Anvin wrote:
>> Create an inline function for clflush(), with the proper arguments,
>> and use it instead of hard-coding the instruction.
>>
>> This also removes one instance of hard-coded wbinvd, based on a patch
>> by Bauder de Oliveira Costa.
> Hey, Who's that guy that got a name so close to mine? ;-)

That would be Mr. Typo!

>> Cc: Andi Kleen <[EMAIL PROTECTED]>
>> Cc: Glauber de Oliveira Costa <[EMAIL PROTECTED]>

I got it right here at least :-/

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-20 Thread Glauber de Oliveira Costa
On Fri, 2007-07-20 at 14:19 -0700, H. Peter Anvin wrote:
> Create an inline function for clflush(), with the proper arguments,
> and use it instead of hard-coding the instruction.
> 
> This also removes one instance of hard-coded wbinvd, based on a patch
> by Bauder de Oliveira Costa.
Hey, Who's that guy that got a name so close to mine? ;-)

> 
> Cc: Andi Kleen <[EMAIL PROTECTED]>
> Cc: Glauber de Oliveira Costa <[EMAIL PROTECTED]>
> Signed-off-by: H. Peter Anvin <[EMAIL PROTECTED]>


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-20 Thread H. Peter Anvin
Andi Kleen wrote:
> On Fri, Jul 20, 2007 at 02:19:58PM -0700, H. Peter Anvin wrote:
>> Create an inline function for clflush(), with the proper arguments,
>> and use it instead of hard-coding the instruction.
>>
>> This also removes one instance of hard-coded wbinvd, based on a patch
>> by Bauder de Oliveira Costa.
> 
> I don't see much sense in it. CLFLUSH is not priviledged, paravirt
> doesn't need to change and this adds just an unnecessary layer of abstraction.

The main reason is that everyone seems to invoke it either incorrectly
or suboptimally.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-20 Thread Andi Kleen
On Fri, Jul 20, 2007 at 02:19:58PM -0700, H. Peter Anvin wrote:
> Create an inline function for clflush(), with the proper arguments,
> and use it instead of hard-coding the instruction.
> 
> This also removes one instance of hard-coded wbinvd, based on a patch
> by Bauder de Oliveira Costa.

I don't see much sense in it. CLFLUSH is not priviledged, paravirt
doesn't need to change and this adds just an unnecessary layer of abstraction.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] x86: Create clflush() inline, remove hardcoded wbinvd

2007-07-20 Thread H. Peter Anvin
Create an inline function for clflush(), with the proper arguments,
and use it instead of hard-coding the instruction.

This also removes one instance of hard-coded wbinvd, based on a patch
by Bauder de Oliveira Costa.

Cc: Andi Kleen <[EMAIL PROTECTED]>
Cc: Glauber de Oliveira Costa <[EMAIL PROTECTED]>
Signed-off-by: H. Peter Anvin <[EMAIL PROTECTED]>

diff --git a/arch/i386/mm/pageattr.c b/arch/i386/mm/pageattr.c
index 37992ff..cb4ded4 100644
--- a/arch/i386/mm/pageattr.c
+++ b/arch/i386/mm/pageattr.c
@@ -70,10 +70,10 @@ static struct page *split_large_page(unsigned long address, 
pgprot_t prot,
 
 static void cache_flush_page(struct page *p)
 { 
-   unsigned long adr = (unsigned long)page_address(p);
+   void *adr = page_address(p);
int i;
for (i = 0; i < PAGE_SIZE; i += boot_cpu_data.x86_clflush_size)
-   asm volatile("clflush (%0)" :: "r" (adr + i));
+   clflush(adr+i);
 }
 
 static void flush_kernel_map(void *arg)
diff --git a/arch/x86_64/kernel/tce.c b/arch/x86_64/kernel/tce.c
index f61fb8e..c01b535 100644
--- a/arch/x86_64/kernel/tce.c
+++ b/arch/x86_64/kernel/tce.c
@@ -40,9 +40,9 @@ static inline void flush_tce(void* tceaddr)
 {
/* a single tce can't cross a cache line */
if (cpu_has_clflush)
-   asm volatile("clflush (%0)" :: "r" (tceaddr));
+   clflush(tceaddr);
else
-   asm volatile("wbinvd":::"memory");
+   wbinvd();
 }
 
 void tce_build(struct iommu_table *tbl, unsigned long index,
diff --git a/arch/x86_64/mm/pageattr.c b/arch/x86_64/mm/pageattr.c
index 9148f4a..863e929 100644
--- a/arch/x86_64/mm/pageattr.c
+++ b/arch/x86_64/mm/pageattr.c
@@ -65,7 +65,7 @@ static void cache_flush_page(void *adr)
 {
int i;
for (i = 0; i < PAGE_SIZE; i += boot_cpu_data.x86_clflush_size)
-   asm volatile("clflush (%0)" :: "r" (adr + i));
+   clflush(adr+i);
 }
 
 static void flush_kernel_map(void *arg)
diff --git a/drivers/char/agp/efficeon-agp.c b/drivers/char/agp/efficeon-agp.c
index df8da72..41f46df 100644
--- a/drivers/char/agp/efficeon-agp.c
+++ b/drivers/char/agp/efficeon-agp.c
@@ -221,7 +221,7 @@ static int efficeon_create_gatt_table(struct 
agp_bridge_data *bridge)
SetPageReserved(virt_to_page((char *)page));
 
for (offset = 0; offset < PAGE_SIZE; offset += clflush_chunk)
-   asm volatile("clflush %0" : : "m" (*(char 
*)(page+offset)));
+   clflush((char *)page+offset);
 
efficeon_private.l1_table[index] = page;
 
@@ -268,15 +268,16 @@ static int efficeon_insert_memory(struct agp_memory * 
mem, off_t pg_start, int t
*page = insert;
 
/* clflush is slow, so don't clflush until we have to */
-   if ( last_page &&
-((unsigned long)page^(unsigned long)last_page) & 
clflush_mask )
-   asm volatile("clflush %0" : : "m" (*last_page));
+   if (last_page &&
+   (((unsigned long)page^(unsigned long)last_page) &
+clflush_mask))
+   clflush(last_page);
 
last_page = page;
}
 
if ( last_page )
-   asm volatile("clflush %0" : : "m" (*last_page));
+   clflush(last_page);
 
agp_bridge->driver->tlb_flush(mem);
return 0;
diff --git a/include/asm-i386/system.h b/include/asm-i386/system.h
index 609756c..d05476d 100644
--- a/include/asm-i386/system.h
+++ b/include/asm-i386/system.h
@@ -160,6 +160,10 @@ static inline void native_wbinvd(void)
asm volatile("wbinvd": : :"memory");
 }
 
+static inline void clflush(volatile void *__p)
+{
+   asm volatile("clflush %0" : "+m" (*(char __force *)__p));
+}
 
 #ifdef CONFIG_PARAVIRT
 #include 
diff --git a/include/asm-x86_64/system.h b/include/asm-x86_64/system.h
index e4f246d..8634067 100644
--- a/include/asm-x86_64/system.h
+++ b/include/asm-x86_64/system.h
@@ -113,6 +113,11 @@ static inline void write_cr4(unsigned long val)
 
 #endif /* __KERNEL__ */
 
+static inline void clflush(volatile void *__p)
+{
+   asm volatile("clflush %0" : "+m" (*(char __force *)__p));
+}
+
 #define nop() __asm__ __volatile__ ("nop")
 
 #ifdef CONFIG_SMP
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/