lkml.org 
[lkml]   [2000]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: spin_lock forgets to clobber memory and other smp fixes [was Re: [patch] waitqueue optimization, 2.4.0-test7]
Andrea Arcangeli wrote:
> >> 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).

Interestingly enough, the local variable case is one where "memory" does
make a difference. Without "memory":

movl p, %eax
movl (%eax), %eax
#APP
#NO_APP

With "memory":

#APP
#NO_APP
movl p, %eax
movl (%eax), %eax

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

As you can see from above, there are cases where

local_var = shared->mumble;
// ...
spin_lock (&spinlock);
local_var = shared->mumble;

requires a "memory" clobber, otherwise the second read, which is in a
critical region, won't be emitted by the compiler.

-- Jamie

ps. There is a _clobber_ for memory, but no way to say "this asm _reads_
arbitrary memory". __volatile__ may be filling that role though.
-
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/

\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.154 / U:30.424 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site