[parisc-linux] Re: [parisc-linux-cvs] linux-2.6 jejb

[John David Anglin]

> > This was rather subtle.  It turns out that gcc was doing a small
> > amount of reordering around the file_lock because it doesn't see our
> > spinlock implementation as being a barrier.  To fix this I

This was in fact reported a number of months ago.  The issue as I now
understand it is that the ldcw macro needs to contain a memory barrier,
so memory stores/loads don't get reordered past it.  I think earlier
versions of GCC didn't reorder past volatile, but from 3.3 on, it is
smarter about these issues.  The macro also doesn't properly indicate
that the lock memory location is modified, if I recall correctly.

I think this would do the trick:

#define __ldcw(a) ({ \
        unsigned __ret; \
	__asm__ __volatile__("ldcw 0(%1),%0" \
			     : "=r" (__ret) : "r" (a) : "memory"); \
	__ret; \
})

The alignment macro is just adjusting a pointer.  GCC should be able to
schedule this code.

The reset of the lock should be made into a macro with a memory
barrier.  It should use a coherent store instead of just "*a = 1".
Something like:

    __asm__ __volatile__ ("stw,ma %1,0(%0)"
			  : : "r" (&lock), "r" (tmp) : "memory");

This does the reset using a coherent store (PA 2.0) and indicates
to GCC that all memory is clobbered.

With these changes, I think you could get rid of the explicit
memory barriers, "mb()", that you currently have.  If you don't
want to include the memory clobbers in these macros, then we should
add clobbers for the lock memory location so GCC knows these locations
are clobbered.

> > SMP now seems stable on a 2xA500 and has survived a 10 hour 35 loop
> > make -j 4 kernel compile without showing any problems (previously, it
> > usually fell over in the first loop).

That's excellent news.

Dave
-- 
J. David Anglin                                  dave.anglin at nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)