You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2002/04/18 14:58:29 UTC
[PATCH] apr_atomic.h on Linux
There are issues with use of asm/atomic.h on Linux: according to the
kernel gurus here this is a header for internal use of the kernel,
and shouldn't be used from userspace.
The problems are, if I understand them correctly:
- with newer kernel/glibc combinations (e.g the RHL7.3 beta) this stuff
really isn't exported to userspace, so they can't be used at all. (this
is the most serious problem, it prevents APR from building)
- it's not a portable assumption to expect anything of this header
merely if "defined(__linux)"; some Linux ports may not contain the right
things. I'd expect it may
- with the current atomic.h I have, the operations aren't SMP-safe
unless you define CONFIG_SMP, so currently if you build on a UP box, it
won't work on an SMP box. (With a 2.2 kernel it looks like you have to
define __SMP__)
I suggest this patch:
--- include/apr_atomic.h 9 Apr 2002 11:13:00 -0000 1.22
+++ include/apr_atomic.h 18 Apr 2002 12:46:11 -0000
@@ -154,24 +154,6 @@
#define apr_atomic_cas(mem,with,cmp) InterlockedCompareExchange(mem,with,cmp)
#define apr_atomic_init(pool) APR_SUCCESS
-#elif defined(__linux)
-
-#include <asm/atomic.h>
-#include <asm/system.h>
-#define apr_atomic_t atomic_t
-
-#define apr_atomic_add(mem, val) atomic_add(val,mem)
-#define apr_atomic_dec(mem) !atomic_dec_and_test(mem)
-#define apr_atomic_inc(mem) atomic_inc(mem)
-#define apr_atomic_set(mem, val) atomic_set(mem, val)
-#define apr_atomic_read(mem) atomic_read(mem)
-#if defined(cmpxchg)
-#define apr_atomic_init(pool) APR_SUCCESS
-#define apr_atomic_cas(mem,with,cmp) cmpxchg(mem,cmp,with)
-#else
-#define APR_ATOMIC_NEED_CAS_DEFAULT 1
-#endif
-
#elif defined(NETWARE)
#include <stdlib.h>
@@ -246,7 +228,7 @@
#define APR_ATOMIC_NEED_CAS_DEFAULT 1
#endif /* APR_HAS_THREADS */
-#endif /* !defined(WIN32) && !defined(__linux) */
+#endif /* !defined(WIN32) */
#if defined(APR_ATOMIC_NEED_DEFAULT)
#define apr_atomic_t apr_uint32_t
The mutex-based apr_atomic.c code looks like it hasn't really been
finished, but with this patch, the "testatomic" passes except for a "no
surprise" result which I presume is supposed to happen. (without the
patch ATOMIC_HASH could be negative, which was... problematic)
--- atomic/unix/apr_atomic.c 9 Apr 2002 06:56:55 -0000 1.12
+++ atomic/unix/apr_atomic.c 18 Apr 2002 12:48:57 -0000
@@ -63,7 +63,7 @@
#define NUM_ATOMIC_HASH 7
/* shift by 2 to get rid of alignment issues */
-#define ATOMIC_HASH(x) (int)(((long)x>>2)%NUM_ATOMIC_HASH)
+#define ATOMIC_HASH(x) (unsigned int)(((unsigned long)(x))%(unsigned int)NUM_ATOMIC_HASH)
static apr_thread_mutex_t **hash_mutex;
apr_status_t apr_atomic_init(apr_pool_t *p )
@@ -92,7 +92,6 @@
apr_thread_mutex_unlock(lock);
/* return prev; */
}
- printf("debug no workee\n");
/* return *mem; */
}
void apr_atomic_set(volatile apr_atomic_t *mem, apr_uint32_t val)
Re: [PATCH] apr_atomic.h on Linux
Posted by Ian Holsman <ia...@apache.org>.
Justin Erenkrantz wrote:
> On Thu, Apr 18, 2002 at 12:36:08PM -0400, Cliff Woolley wrote:
>
>>On Thu, 18 Apr 2002, Joe Orton wrote:
>>
>>
>>>Sure - I'd just hope the change could be made for the time being, until
>>>someone gets round to that, since currently APR won't build on
>>>combinations of modern glibc/kernel.
>>
hmm.. 36-dev is building ok for me.
[root@spark moz]# cat /proc/version
Linux version 2.4.18-0.4 (bhcompile@daffy.perf.redhat.com) (gcc version
2.96 20000731 (Red Hat Linux 7.2 2.96-106)) #1 Wed Mar 13 10:14:50 EST 2002
but the testatomic program is segfaulting ;(
>>Which combinations? It works fine for me on glibc 2.2.5 / linux 2.4.18.
>>Granted, I use slackware-current and not RH. :)
>
>
> I'm not seeing a problem here either with Mandrake Cooker. -- justin
>
Re: [PATCH] apr_atomic.h on Linux
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Apr 18, 2002 at 09:59:49AM -0700, Justin Erenkrantz wrote:
> On Thu, Apr 18, 2002 at 12:36:08PM -0400, Cliff Woolley wrote:
> > On Thu, 18 Apr 2002, Joe Orton wrote:
> >
> > > Sure - I'd just hope the change could be made for the time being, until
> > > someone gets round to that, since currently APR won't build on
> > > combinations of modern glibc/kernel.
> >
> > Which combinations? It works fine for me on glibc 2.2.5 / linux 2.4.18.
> > Granted, I use slackware-current and not RH. :)
>
> I'm not seeing a problem here either with Mandrake Cooker. -- justin
Ah, sorry, I hadn't understood this part properly, it's apparently a
product of our packaging being a little ahead of upstream.
An additional point which was made to me was that using inline functions
from a GPL'ed header (as kernel headers tend to be) has interesting
licensing consequences, if you still need convincing...
joe
Re: [PATCH] apr_atomic.h on Linux
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, Apr 18, 2002 at 12:36:08PM -0400, Cliff Woolley wrote:
> On Thu, 18 Apr 2002, Joe Orton wrote:
>
> > Sure - I'd just hope the change could be made for the time being, until
> > someone gets round to that, since currently APR won't build on
> > combinations of modern glibc/kernel.
>
> Which combinations? It works fine for me on glibc 2.2.5 / linux 2.4.18.
> Granted, I use slackware-current and not RH. :)
I'm not seeing a problem here either with Mandrake Cooker. -- justin
Re: [PATCH] apr_atomic.h on Linux
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 18 Apr 2002, Joe Orton wrote:
> Sure - I'd just hope the change could be made for the time being, until
> someone gets round to that, since currently APR won't build on
> combinations of modern glibc/kernel.
Which combinations? It works fine for me on glibc 2.2.5 / linux 2.4.18.
Granted, I use slackware-current and not RH. :)
--Cliff
Re: [PATCH] apr_atomic.h on Linux
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Apr 18, 2002 at 08:38:31AM -0700, Brian Pane wrote:
> Joe Orton wrote:
>
> >There are issues with use of asm/atomic.h on Linux: according to the
> >kernel gurus here this is a header for internal use of the kernel,
> >and shouldn't be used from userspace.
> >
> >The problems are, if I understand them correctly:
> >
> >- with newer kernel/glibc combinations (e.g the RHL7.3 beta) this stuff
> >really isn't exported to userspace, so they can't be used at all. (this
> >is the most serious problem, it prevents APR from building)
> >
>
> Thanks for reporting this. I agree that we need to stop relying on
> asm/atomic.h on Linux.
>
> My only objection to the patch is that it solves the problem by switching
> to the mutex-based default implementation on Linux--which defeats the whole
> point of the API on one of our most important platforms. I suppose we can
> fix that by using inline assembly to generate the atomic operations for the
> Linux+gcc+x86 case.
Sure - I'd just hope the change could be made for the time being, until
someone gets round to that, since currently APR won't build on
combinations of modern glibc/kernel.
Regards,
joe
Re: [PATCH] apr_atomic.h on Linux
Posted by Brian Pane <bp...@pacbell.net>.
Thom May wrote:
>>My only objection to the patch is that it solves the problem by switching
>>to the mutex-based default implementation on Linux--which defeats the whole
>>point of the API on one of our most important platforms. I suppose we can
>>fix that by using inline assembly to generate the atomic operations for the
>>Linux+gcc+x86 case.
>>
>Which is great for the x86 case, but neglects the other 10+ architectures
>that linux runs on (including things like IA64, HP's PA-RISC, UltraSparc,
>PowerPC etc).
>
>Debian 3.0 is gonna be available (May 1, hopefully) on 11 architectures, and
>certainly people would expect APR to be able to support all of them the same
>- are you proposing to keep inline assembler in APR for all these cases?
>
Of course not. The platforms for which people contribute and maintain
real atomic operations will have optimal implementations, and all other
platforms will have the (functional but slower) mutex-based default.
--Brian
Re: [PATCH] apr_atomic.h on Linux
Posted by Thom May <th...@planetarytramp.net>.
* Brian Pane (brian.pane@cnet.com) wrote :
> Joe Orton wrote:
>
> >There are issues with use of asm/atomic.h on Linux: according to the
> >kernel gurus here this is a header for internal use of the kernel,
> >and shouldn't be used from userspace.
> >
> >The problems are, if I understand them correctly:
> >
> >- with newer kernel/glibc combinations (e.g the RHL7.3 beta) this stuff
> >really isn't exported to userspace, so they can't be used at all. (this
> >is the most serious problem, it prevents APR from building)
> >
>
> Thanks for reporting this. I agree that we need to stop relying on
> asm/atomic.h on Linux.
>
> My only objection to the patch is that it solves the problem by switching
> to the mutex-based default implementation on Linux--which defeats the whole
> point of the API on one of our most important platforms. I suppose we can
> fix that by using inline assembly to generate the atomic operations for the
> Linux+gcc+x86 case.
>
Which is great for the x86 case, but neglects the other 10+ architectures
that linux runs on (including things like IA64, HP's PA-RISC, UltraSparc,
PowerPC etc).
Debian 3.0 is gonna be available (May 1, hopefully) on 11 architectures, and
certainly people would expect APR to be able to support all of them the same
- are you proposing to keep inline assembler in APR for all these cases?
(apache | 1.3.24-2.1 | alpha, arm, hppa, i386, ia64, m68k, mips, mipsel, powerpc, s390, sparc)
Cheers,
-Thom
--
Thom May -> thom@planetarytramp.net
<aj> *sigh* you'd think a distribution composed of 6000
packages distributed across 13 different architectures (in various stages
between pre-alpha and release quality), maintained by 700 amateurs with often
conflicting goals who're globally distributed and have rarely met each other
-- you'd think a distribution like that would be simpler...
Re: [PATCH] apr_atomic.h on Linux
Posted by Brian Pane <br...@cnet.com>.
Joe Orton wrote:
>There are issues with use of asm/atomic.h on Linux: according to the
>kernel gurus here this is a header for internal use of the kernel,
>and shouldn't be used from userspace.
>
>The problems are, if I understand them correctly:
>
>- with newer kernel/glibc combinations (e.g the RHL7.3 beta) this stuff
>really isn't exported to userspace, so they can't be used at all. (this
>is the most serious problem, it prevents APR from building)
>
Thanks for reporting this. I agree that we need to stop relying on
asm/atomic.h on Linux.
My only objection to the patch is that it solves the problem by switching
to the mutex-based default implementation on Linux--which defeats the whole
point of the API on one of our most important platforms. I suppose we can
fix that by using inline assembly to generate the atomic operations for the
Linux+gcc+x86 case.
--Brian
Re: [PATCH] apr_atomic.h on Linux
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Apr 18, 2002 at 01:58:29PM +0100, Joe Orton wrote:
> There are issues with use of asm/atomic.h on Linux: according to the
> kernel gurus here this is a header for internal use of the kernel,
> and shouldn't be used from userspace.
>
> The problems are, if I understand them correctly:
>
> - with newer kernel/glibc combinations (e.g the RHL7.3 beta) this stuff
> really isn't exported to userspace, so they can't be used at all. (this
> is the most serious problem, it prevents APR from building)
>
> - it's not a portable assumption to expect anything of this header
> merely if "defined(__linux)"; some Linux ports may not contain the right
> things. I'd expect it may
... not work on old versions of Linux either, I think I meant to say
there :)
joe
Re: [PATCH] apr_atomic.h on Linux
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 18 Apr 2002, Justin Erenkrantz wrote:
> I committed this second part as with that testatomic passes here
> on my Linux/SMP box.
It passed for me beforehand on my non-SMP linux box, but it still passes
afterward, so +1 on that commit.
--Cliff
Re: [PATCH] apr_atomic.h on Linux
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, Apr 18, 2002 at 01:58:29PM +0100, Joe Orton wrote:
> The mutex-based apr_atomic.c code looks like it hasn't really been
> finished, but with this patch, the "testatomic" passes except for a "no
> surprise" result which I presume is supposed to happen. (without the
> patch ATOMIC_HASH could be negative, which was... problematic)
I committed this second part as with that testatomic passes here
on my Linux/SMP box.
I'll leave the first part up to discussion. -- justin