Re: spin_lock forgets to clobber memory and other smp fixes [was Re: [patch] waitqueue optimization, 2.4.0-test7]

From: Andrea Arcangeli (andrea@suse.de)
Date: Thu Sep 07 2000 - 11:21:41 EST


On Thu, 7 Sep 2000, Jamie Lokier wrote:

>asm *__volatile__* seems to make no difference. I've tried a few things.

It makes a difference, see below.

>
>Andrea Arcangeli wrote:
>> Maybe we can rely on the __volatile__ statement of the asm that will
>> enforce that if we write:
>>
>> *p = 0;
>> __asm__ __volatile__("" : :);
>> *p = 1;
>>
>> in the assembler we'll then find both a write of 0 and then a write of 1
>> to memory.
>
>That does 2 writes with gcc-2.96 and also egcs-2.91.66/19990314
>(Red Hat's kgcc), with or without -fstrict-aliasing.
>
>It also does 2 writes without __volatile__.
>
>> int a = *p;
>> __asm__ __volatile__("" : :);
>> a = *p;
>>
>> (to do two explicit reads)
>
>Sorry, that does just one read, kgcc (old stable gcc) and also with
>gcc-2.96. Type aliasing on/off makes no difference to the number of reads.

I wrote the above not just as a complete testecase, but just to mean what
the case I was talking about. You made int a a local variable and the
thing you noticed is an otimization that the compiler is allowed to do
regardless of the "memory" clobber too (`int a' have to be at least extern
otherwise the compiler understands the first read can go away). Try this:

int * p;
int a;

extern f(int);

main()
{
        a = *p;
        
        __asm__ __volatile__("zzz" : : );

        a = *p;

        f(a);
}

Try to add "memory" as clobber to the above testcase and nothing will
change. (that's what I meant in my previous email saying that even w/o
"memory" things got compiled right at least in my simple testcases)

>Again, __volatile__ makes no difference.

Note that __volatile__ really makes a difference if for example you
speficy as output an operand that isn't used anymore.

Try this:

main()
{
        int a;
        __asm__("zzz" : "=r" (a));
}

and then this:

main()
{
        int a;
        __asm__ __volatile__("zzz" : "=r" (a));
}

--- p.s.nonvolatile Thu Sep 7 18:05:30 2000
+++ p.s Thu Sep 7 18:05:53 2000
@@ -6,10 +6,13 @@
 .globl main
         .type main,@function
 main:
         pushl %ebp
         movl %esp,%ebp
+#APP
+ zzz
+#NO_APP
         movl %ebp,%esp
         popl %ebp
         ret
 .Lfe1:
         .size main,.Lfe1-main

>I cannot tell from the GCC manual whether either of these behaviours is
>correct or not.

The behaviour of what you described is definitely correct/safe.

My only wondering was about "memory" non-"memory" as clobber because gcc
was doing things right just with the __asm__ __volatile__ thing w/o
"memory" as clobber. However I had the confirm my worries was right and
that "memory" is needed for all the spinlocks.

BTW, we had a bug in the alpha port last month in the
linux/include/asm-alpha/fpu.h:wrfpcr() function, read it for a real world
example of where __volatile__ must be used.

Andrea

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



This archive was generated by hypermail 2b29 : Thu Sep 07 2000 - 21:00:30 EST