Thread: S_LOCK() change produces error...
I installed some patches today for the univel port, and one of the changes
did the following to include/storage/s_lock.h:
302c318
< __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
---
> __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
Under FreeBSD, this breaks the compile with an 'unimplemented' error...I'm
not going to commit the "fix" yet (reverse the patch)...does this break other
ports as well, or just FreeBSD? Anyone with Assembler experience out there? :)
Marc G. Fournier
Systems Administrator @ hub.org
primary: scrappy@hub.org secondary: scrappy@{freebsd|postgresql}.org
>
>
> I installed some patches today for the univel port, and one of the changes
> did the following to include/storage/s_lock.h:
>
> 302c318
> < __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> ---
> > __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
>
I guess this is a multiple cpu modifier for asm, and most people don't
run multiple cpus. I guess our gcc's call it an error, rather than
ignore it. I think we need an OS-specific ifdef there. We can't have
Univel changing the normal i386 stuff that works so well now.
--
Bruce Momjian
maillist@candle.pha.pa.us
On Sat, 17 Jan 1998, Bruce Momjian wrote:
> >
> >
> > I installed some patches today for the univel port, and one of the changes
> > did the following to include/storage/s_lock.h:
> >
> > 302c318
> > < __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > ---
> > > __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> >
>
> I guess this is a multiple cpu modifier for asm, and most people don't
> run multiple cpus. I guess our gcc's call it an error, rather than
> ignore it. I think we need an OS-specific ifdef there. We can't have
> Univel changing the normal i386 stuff that works so well now.
Actually, I think that the patch was meant to improve...if you look at the
code, he put all the Univel stuff inside of its own #ifdef...see around
line 297 in include/storage/s_lock.h and you'll see what I mean.
He seems to have only added a 'lock' to the beginning of the __asm__,
which is what is breaking things under FreeBSD, but unless it affects every
other port, I'm loath to remove it without just throwing in a FreeBSD #ifdef
in there...
Marc G. Fournier
Systems Administrator @ hub.org
primary: scrappy@hub.org secondary: scrappy@{freebsd|postgresql}.org
>
> On Sat, 17 Jan 1998, Bruce Momjian wrote:
>
> > >
> > >
> > > I installed some patches today for the univel port, and one of the changes
> > > did the following to include/storage/s_lock.h:
> > >
> > > 302c318
> > > < __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > > ---
> > > > __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > >
> >
> > I guess this is a multiple cpu modifier for asm, and most people don't
> > run multiple cpus. I guess our gcc's call it an error, rather than
> > ignore it. I think we need an OS-specific ifdef there. We can't have
> > Univel changing the normal i386 stuff that works so well now.
>
> Actually, I think that the patch was meant to improve...if you look at the
> code, he put all the Univel stuff inside of its own #ifdef...see around
> line 297 in include/storage/s_lock.h and you'll see what I mean.
>
> He seems to have only added a 'lock' to the beginning of the __asm__,
> which is what is breaking things under FreeBSD, but unless it affects every
> other port, I'm loath to remove it without just throwing in a FreeBSD #ifdef
> in there...
I will check when I apply my next patch. I am hesitant to cvsup at this
time if the code is broken.
--
Bruce Momjian
maillist@candle.pha.pa.us
On Sat, 17 Jan 1998, The Hermit Hacker wrote:
> On Sat, 17 Jan 1998, Bruce Momjian wrote:
> > > I installed some patches today for the univel port, and one of the changes
> > > did the following to include/storage/s_lock.h:
> > >
> > > 302c318
> > > < __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > > ---
> > > > __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> >
> > I guess this is a multiple cpu modifier for asm, and most people don't
> > run multiple cpus. I guess our gcc's call it an error, rather than
> > ignore it. I think we need an OS-specific ifdef there. We can't have
> > Univel changing the normal i386 stuff that works so well now.
>
> Actually, I think that the patch was meant to improve...if you look at the
> code, he put all the Univel stuff inside of its own #ifdef...see around
> line 297 in include/storage/s_lock.h and you'll see what I mean.
>
> He seems to have only added a 'lock' to the beginning of the __asm__,
> which is what is breaking things under FreeBSD, but unless it affects every
> other port, I'm loath to remove it without just throwing in a FreeBSD #ifdef
> in there...
(clip from SMP support in linux' asm/spinlocks.h)
#define spin_unlock(lock) \
__asm__ __volatile__( \
"lock ; btrl $0,%0" \
:"=m" (__dummy_lock(lock)))
in linux the lock has ";" following.
Yep - it's for multiCPU systems (SMP). Handy for shared-memory systems
too if you're really into multithreading-speed.
It locks that particular byte (word?) of memory against access by other
CPU's accessing it IIRC...
Perhaps your GAS is too old? (GNU binutils)
(does BSD support multiple CPU's under intel?)
multiprocessor really isn't that rare under linux - even Linus Torvalds
uses a SMP system *grin*...
Maybe he encountered a locking problem with a multicpu host and needed a
semaphore (or equiv) to lock things? Just trying to figure this out...
(sometimes necessary if you're doing shared memory across processes)
G'day, eh? :)
- Teunis
>
> On Sat, 17 Jan 1998, The Hermit Hacker wrote:
>
> > On Sat, 17 Jan 1998, Bruce Momjian wrote:
> > > > I installed some patches today for the univel port, and one of the changes
> > > > did the following to include/storage/s_lock.h:
> > > >
> > > > 302c318
> > > > < __asm__("xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > > > ---
> > > > > __asm__("lock xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(0x1)); \
> > >
> > > I guess this is a multiple cpu modifier for asm, and most people don't
> > > run multiple cpus. I guess our gcc's call it an error, rather than
> > > ignore it. I think we need an OS-specific ifdef there. We can't have
> > > Univel changing the normal i386 stuff that works so well now.
> >
> > Actually, I think that the patch was meant to improve...if you look at the
> > code, he put all the Univel stuff inside of its own #ifdef...see around
> > line 297 in include/storage/s_lock.h and you'll see what I mean.
> >
> > He seems to have only added a 'lock' to the beginning of the __asm__,
> > which is what is breaking things under FreeBSD, but unless it affects every
> > other port, I'm loath to remove it without just throwing in a FreeBSD #ifdef
> > in there...
>
> (clip from SMP support in linux' asm/spinlocks.h)
> #define spin_unlock(lock) \
> __asm__ __volatile__( \
> "lock ; btrl $0,%0" \
> :"=m" (__dummy_lock(lock)))
>
> in linux the lock has ";" following.
> Yep - it's for multiCPU systems (SMP). Handy for shared-memory systems
> too if you're really into multithreading-speed.
>
> It locks that particular byte (word?) of memory against access by other
> CPU's accessing it IIRC...
>
> Perhaps your GAS is too old? (GNU binutils)
> (does BSD support multiple CPU's under intel?)
>
> multiprocessor really isn't that rare under linux - even Linus Torvalds
> uses a SMP system *grin*...
>
> Maybe he encountered a locking problem with a multicpu host and needed a
> semaphore (or equiv) to lock things? Just trying to figure this out...
> (sometimes necessary if you're doing shared memory across processes)
Marc, I will try 'lock;' and if it works, will submit a patch.
--
Bruce Momjian
maillist@candle.pha.pa.us
> > (clip from SMP support in linux' asm/spinlocks.h) > > #define spin_unlock(lock) \ > > __asm__ __volatile__( \ > > "lock ; btrl $0,%0" \ > > :"=m" (__dummy_lock(lock))) > > > > in linux the lock has ";" following. > > Yep - it's for multiCPU systems (SMP). Handy for shared-memory systems > > too if you're really into multithreading-speed. > > > > It locks that particular byte (word?) of memory against access by other > > CPU's accessing it IIRC... > > > > Perhaps your GAS is too old? (GNU binutils) > > (does BSD support multiple CPU's under intel?) > > > > multiprocessor really isn't that rare under linux - even Linus Torvalds > > uses a SMP system *grin*... > > > > Maybe he encountered a locking problem with a multicpu host and needed a > > semaphore (or equiv) to lock things? Just trying to figure this out... > > (sometimes necessary if you're doing shared memory across processes) > > Marc, I will try 'lock;' and if it works, will submit a patch. Yep, it works. Patch applied. -- Bruce Momjian maillist@candle.pha.pa.us