Re: ldcw inline assembler patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 14, 2008 at 11:36 AM, Helge Deller <deller@xxxxxx> wrote:
> I'm wondering if this patch might help people who are seeing locking
> problems on SMP boxes ?
> Helge
>
> diff --git a/include/asm-parisc/system.h b/include/asm-parisc/system.h
> index ee80c92..4752684 100644
> --- a/include/asm-parisc/system.h
> +++ b/include/asm-parisc/system.h
> @@ -168,8 +168,9 @@ static inline void set_eiem(unsigned long val)
>  /* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.  */
>  #define __ldcw(a) ({                                           \
>        unsigned __ret;                                         \
> -       __asm__ __volatile__(__LDCW " 0(%1),%0"                 \
> -               : "=r" (__ret) : "r" (a));                      \
> +       __asm__ __volatile__(__LDCW " 0(%2),%0"                 \
> +                       : "=r" (__ret), "=m" (*(a))             \
> +                       : "r" (a), "m" (*(a))   );              \
>        __ret;                                                  \
>  })

You don't want to do that, the compiler might hold "=m" (*(a)) in a
temporary memory location.

e.g.
temp_mem = *a;
reg2 = &temp_mem;
... operation ...
*a = temp_mem;

You would be atomic with regards to the store to temp_mem, but not a.
Infact this could always be the case.

I'm now of the opinion that we need a "memory" clobber in the original
expression to prevent this from ever happening.

Cheers,
Carlos.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux