This is the mail archive of the
libc-alpha@sourceware.cygnus.com
mailing list for the glibc project.
Re: [drow@false.org: Re: DB_THREAD support in Berkeley DB/glibc]
- To: drow at false dot org
- Subject: Re: [drow@false.org: Re: DB_THREAD support in Berkeley DB/glibc]
- From: Geoff Keating <geoffk at cygnus dot com>
- Date: Tue, 28 Dec 1999 15:30:25 -0800
- CC: libc-alpha at sourceware dot cygnus dot com
- References: <19991228171708.A19114@drow.res.cmu.edu>
> Mailing-List: contact libc-alpha-help@sourceware.cygnus.com; run by ezmlm
> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-geoffk=cygnus.com@sourceware.cygnus.com>
> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.cygnus.com>
> List-Archive: <http://sourceware.cygnus.com/ml/libc-alpha/>
> List-Post: <mailto:libc-alpha@sourceware.cygnus.com>
> List-Help: <mailto:libc-alpha-help@sourceware.cygnus.com>, <http://sourceware.cygnus.com/ml/#faqs>
> Date: Tue, 28 Dec 1999 17:17:08 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Mail-Followup-To: libc-alpha@sourceware.cygnus.com
> User-Agent: Mutt/1.0i
>
> ----- Forwarded message from Daniel Jacobowitz <drow@false.org> -----
>
> Date: Tue, 28 Dec 1999 17:05:39 -0500
> From: Daniel Jacobowitz <drow@false.org>
> To: linuxppc-dev@lists.linuxppc.org, libc-alpha@cygnus.com
> Subject: Re: DB_THREAD support in Berkeley DB/glibc
> Mail-Followup-To: linuxppc-dev@lists.linuxppc.org, libc-alpha@cygnus.com
>
> I'm fixing this up right now... From what I could see, the tsl_t is a
> char but this accesses it as a long; dare I just assume that struct
> alignment will do the right thing? My current code conditionally sets
> tsl_t to an unsigned long on powerpc, via sysdeps.
No, it should be a 'long' (actually, an 'int'). Changing tsl_t is right.
If you _can't_ change tsl_t, you need to do something like:
#define TSL_SET(tsl) ({
unsigned int *aligned_tsl = (unsigned int *)((uintptr_t)(tsl) & ~3);
int shiftamount = 24 - ((uintptr_t)(tsl) & 3)*8;
unsigned int mask = 0xFF << shiftamount;
unsigned int ones = 0x01 << shiftamount;
unsigned int result, temp;
__asm__ ("
0: lwarx %0,0,%2
and. %1,%0,%3
bne+ 1f
or %0,%0,%4
stwcx. %0,0,%2
bne- 0b
1:" : "=&r"(temp), "=&r"(result)
: "r"(aligned_tsl), "r"(mask), "r"(ones)
: "cr0", "memory"); !result; })
[that's completely untested code, and you need to stick 'sync' in at
the right places...]
but this can cause livelock, so it's good to avoid it.
If you know that 'tsl' is always aligned properly (to a cache line
boundary), you alway avoid livelock and can simplify this code quite a
bit. It becomes
#define TSL_SET(tsl) ({
unsigned int result, temp;
__asm__ ("
0: lwarx %0,0,%2
clrrwi. %1,%0,24
bne+ 1f
oris %0,%0,0x0100
stwcx. %0,0,%2
bne- 0b
1:" : "=&r"(temp), "=&r"(result)
: "r"(tsl)
: "cr0", "memory"); !result; })
but if 'tsl' ever crosses a cache line, you will get a signal.
> And no, UNSET is not sufficient, from what I can tell - at least on
> SMP.
>
> I used sync at both ends, by analogy to the mutexes for linuxthreads;
> is this wrong/unnecessary?
It's not wrong. It may be unnecessary.
> Does this implementation look right? I'm away from my powerpc at the
> moment, so I haven't even been able to verify that it compiles.
>
>
> On Tue, Dec 28, 1999 at 02:02:03PM -0500, David Edelsohn wrote:
> >
> > +/*
> > + * PowerPC spinlock, adapted from the Alpha and m68k ones by dhd@debian.org
> > + *
> > + * For gcc/powerpc, 0 is clear, 1 is set (but *tsl will always be 0 since it's a char)
> > + */
> > +#define TSL_SET(tsl) ({ \
> > + register tsl_t *__l = (tsl); \
> > + register tsl_t __r1; \
> > + __asm__ volatile(" \n\
> > + 10: lwarx %0,0,%1 \n\
> > + cmpwi %0,0 \n\
> > + bne+ 20f \n\
> > + stwcx. %2,0,%1 \n\
> > + bne- 10b \n\
> > + 20: " \
> > + : "=&r" (__r1) \
> > + : "r" (__l), "r" (-1) : "cr0", "memory"); \
> > + !__r1; \
> > +})
> > +
> > +#define TSL_UNSET(tsl) (*(tsl) = 0)
> > +#define TSL_INIT(tsl) TSL_UNSET(tsl)
> >
> > The TSL_SET macro is basically correct for PowerPC uniprocessor,
> > but it is not MP safe. For cases where this needs to be safe across a
> > multiprocessor complex, it should be preceded by a "sync" instruction and
> > ended with an "isync" instruction, or something similar depending on the
> > semantics one uses for accessing the word.
> >
> > It is not clear to me why the TSL_UNSET macro is sufficient.
>
>
> Dan
>
> /--------------------------------\ /--------------------------------\
> | Daniel Jacobowitz |__| SCS Class of 2002 |
> | Debian GNU/Linux Developer __ Carnegie Mellon University |
> | dan@debian.org | | dmj+@andrew.cmu.edu |
> \--------------------------------/ \--------------------------------/
[lots of stuff deleted.]
> --- db2.orig/mutex/powerpc.gcc Wed Dec 31 19:00:00 1969
> +++ db2/mutex/powerpc.gcc Tue Dec 28 16:57:31 1999
> @@ -0,0 +1,31 @@
> +/*
> + * PowerPC spinlock, adapted from the Alpha and m68k ones by dhd@debian.org
> + * and dan@debian.org.
> + *
> + * For gcc/powerpc, 0 is clear, 1 is set.
> + */
> +#define TSL_SET(tsl) ({ \
> + register tsl_t *__l = (tsl); \
> + register tsl_t __r1; \
> + __asm__ volatile(" \n\
> + sync \n\
> + 10: lwarx %0,0,%1 \n\
> + cmpwi %0,0 \n\
> + bne+ 20f \n\
> + stwcx. %2,0,%1 \n\
> + bne- 10b \n\
> + sync \n\
> + 20: " \
> + : "=&r" (__r1) \
> + : "r" (__l), "r" (1) : "cr0", "memory"); \
> + !__r1; \
> +})
You don't need to say 'volatile' here. The 'memory' clobber handles
it.
> +
> +#define TSL_UNSET(tsl) ({ \
> + register tsl_t *__l = (tsl); \
> + __asm__ __volatile__(" \n\
> + sync \n\
> + stw %1, %0" : "=&r" (__l) : "r" (0)); \
> + })
This won't work. You do need the 'memory' clobber. You want
#define TSL_UNSET(tsl) do { \
__asm__ ("sync" : : : "memory"); \
*tsl = 0; \
} while (0)
This will ensure that any memory stores that were made before the
'sync' are visible before *tsl is reset to zero. If it is also
necessary that the change to *tsl is visible before any subsequent
stores, you can duplicate the 'asm' after the store.
> +#define TSL_INIT(tsl) TSL_UNSET(tsl)
> diff -uNr sysdeps/powerpc.orig/Makefile sysdeps/powerpc/Makefile
> --- sysdeps/powerpc.orig/Makefile Tue Dec 28 15:50:56 1999
> +++ sysdeps/powerpc/Makefile Tue Dec 28 15:53:01 1999
> @@ -25,6 +25,10 @@
> CFLAGS-gmon-start.o = -G0
> endif
>
> +ifeq ($(subdir),db2)
> +CPPFLAGS += -DHAVE_SPINLOCKS=1 -DHAVE_ASSEM_POWERPC_GCC=1 -DSPINLOCK_TYPE='unsigned long' -DMUTEX_ALIGNMENT=4
> +endif
> +
You need MUTEX_ALIGNMENT=32 for powerpc.
--
- Geoffrey Keating <geoffk@cygnus.com>