You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Davide Berti <db...@yahoo.com> on 2002/05/23 19:28:08 UTC
QNX 6.1a mod/peer review
Hello all
httpd-2.0.36, QNX 6.1 RTOS
I have made a modification to apache to get it to run
on qnx. It kept hanging. I traced it down and made
some changes to get it to run. I wanted to proof
these changes with the apache community and get some
feedback as to the longterm consequences/ side effects
of these changes. I am attaching a diff of the
changes that I made.
I also noticed that SSL doesn't seem to work unless
the -X flag is passed to httpd, any ideas.
Thanks
/Davide
--- httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
Mon Apr 8 23:56:56 2002
+++ ../httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
Wed May 8 16:04:51 2002
@@ -318,7 +318,9 @@
if (munmap((caddr_t)mutex->pthread_interproc,
sizeof(pthread_mutex_t))){
return errno;
}
- }
+ if(shm_unlink("/datapoints")) // DB
+ return errno;
+ }
return APR_SUCCESS;
}
@@ -329,11 +331,15 @@
int fd;
pthread_mutexattr_t mattr;
- fd = open("/dev/zero", O_RDWR);
- if (fd < 0) {
- return errno;
- }
+ fd=shm_open("/datapoints",O_RDWR|O_CREAT,0777);
//DB
+ if (fd < 0)
+ return errno;
+ if(ftruncate(fd,sizeof(pthread_mutex_t))==-1) //DB
+ return errno;
+
+
+
new_mutex->pthread_interproc = (pthread_mutex_t
*)mmap(
(caddr_t) 0,
sizeof(pthread_mutex_t),
@@ -363,10 +369,11 @@
PTHREAD_MUTEX_ROBUST_NP))) {
#ifdef PTHREAD_SETS_ERRNO
rv = errno;
-#endif
+#endif // DB
proc_mutex_proc_pthread_cleanup(new_mutex);
return rv;
}
+#endif
if ((rv = pthread_mutexattr_setprotocol(&mattr,
PTHREAD_PRIO_INHERIT))) {
#ifdef PTHREAD_SETS_ERRNO
rv = errno;
@@ -374,9 +381,15 @@
proc_mutex_proc_pthread_cleanup(new_mutex);
return rv;
}
+ if ((rv =
pthread_mutex_destroy(new_mutex->pthread_interproc)))
{ // DB
+#ifdef PTHREAD_SETS_ERRNO
+ rv = errno;
#endif
+ proc_mutex_proc_pthread_cleanup(new_mutex);
+ return rv;
+ }
- if ((rv =
pthread_mutex_init(new_mutex->pthread_interproc,
&mattr))) {
+ if ((rv =
pthread_mutex_init(new_mutex->pthread_interproc,
&mattr))) {
#ifdef PTHREAD_SETS_ERRNO
rv = errno;
#endif
__________________________________________________
Do You Yahoo!?
LAUNCH - Your Yahoo! Music Experience
http://launch.yahoo.com
Re: QNX 6.1a mod/peer review
Posted by Igor Kovalenko <Ig...@motorola.com>.
Anyone? ping! Or should i resumbit once a week as Justin suggested once ;)
----- Original Message -----
From: "Igor Kovalenko" <Ig...@motorola.com>
To: <de...@httpd.apache.org>
Sent: Wednesday, August 07, 2002 2:24 PM
Subject: Re: QNX 6.1a mod/peer review
> Was that applied or not? Does not seem to be in 2.0.39...
>
> As for the content of patch, it appears to do 2 things: replace mmmap() of
> /dev/zero with mmap() of a regular file (which is not the best idea) and
> handle retuns of pthread_xxx() funcs differently (they return errno value
> rather than set errno on QNX6). This all applies to
> USE_PROC_PTHREAD_SERIALIZE code.
>
> However, USE_PROC_PTHREAD_SERIALIZE is not defined on QNX6. The
> USE_PTHREAD_SERIALIZE is defined, but useless since apache has no code for
> it beyond handling -V option. USE_POSIXSEM_SERIALIZE is defined too and
> SINGLE_LISTEN_UNSERIALIZED_ACCEPT is also defined.
>
> Which means (as far as i can tell) USE_POSIXSEM_SERIALIZE will be used if
> more than one Listen is present. That code looks somewhat strange to me
too.
> Is there a reason why named POSIX semaphores are used? Named ones add lot
of
> unnecessary trouble (there are even comments inline about joy of using
them)
> and on QNX they are also whole lot slower. Why not put unnamed ones into
> anon shared memory? Is this code used by any other platform which does
have
> named POSIX sems but does not have anon shared memory? The time_sem.c does
> not have code to test POSIXSEM_SERIALIZE performance. I did test
FLOCK/FCNTL
> and PTHREAD though and PTHREAD is 5-6 times faster on QNX6 (single CPU, 50
> children, 10000 iterations).
>
> So here is what needs to be done I think:
>
> 1. define USE_PROC_PTHREAD_SERIALIZE for QNX6
> 2. patch USE_PROC_PTHREAD_SERIALIZE to use mmap(MAP_ANON) is available and
> fallback to mmap(/dev/zero) otherwise; also patch it to use QNX6
equivalent
> of pthread_mutex_setconsistent_np().
> 3. patch for handling of pthread_xxx() returns.
> 4. handle scoreboard code to use mmap() in similar way to (2)
> 5. remove QNX6 from list of systems which don't have initgroups().
>
> Are there upfront objections to such patch before I bother doing it? There
> were objections when I tried to do (2) for apache1.3 - it was said to be
> non-QNX-specific. My arguments (that it actually should be
non-QNX-specific)
> were never answered and patch was summarily ignored.
>
> Regards,
> -- igor
>
> ----- Original Message -----
> From: "Davide Berti" <db...@yahoo.com>
> To: <de...@httpd.apache.org>
> Sent: Thursday, May 23, 2002 12:28 PM
> Subject: QNX 6.1a mod/peer review
>
>
> > Hello all
> >
> > httpd-2.0.36, QNX 6.1 RTOS
> >
> > I have made a modification to apache to get it to run
> > on qnx. It kept hanging. I traced it down and made
> > some changes to get it to run. I wanted to proof
> > these changes with the apache community and get some
> > feedback as to the longterm consequences/ side effects
> > of these changes. I am attaching a diff of the
> > changes that I made.
> >
> > I also noticed that SSL doesn't seem to work unless
> > the -X flag is passed to httpd, any ideas.
> >
> > Thanks
> > /Davide
> >
> > --- httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> > Mon Apr 8 23:56:56 2002
> > +++ ../httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> > Wed May 8 16:04:51 2002
> > @@ -318,7 +318,9 @@
> > if (munmap((caddr_t)mutex->pthread_interproc,
> > sizeof(pthread_mutex_t))){
> > return errno;
> > }
> > - }
> > + if(shm_unlink("/datapoints")) // DB
> > + return errno;
> > + }
> > return APR_SUCCESS;
> > }
> >
> > @@ -329,11 +331,15 @@
> > int fd;
> > pthread_mutexattr_t mattr;
> >
> > - fd = open("/dev/zero", O_RDWR);
> > - if (fd < 0) {
> > - return errno;
> > - }
> > + fd=shm_open("/datapoints",O_RDWR|O_CREAT,0777);
> > file://DB
> > + if (fd < 0)
> > + return errno;
> >
> > + if(ftruncate(fd,sizeof(pthread_mutex_t))==-1) file://DB
> > + return errno;
> > +
> > +
> > +
> > new_mutex->pthread_interproc = (pthread_mutex_t
> > *)mmap(
> > (caddr_t) 0,
> >
> > sizeof(pthread_mutex_t),
> > @@ -363,10 +369,11 @@
> >
> > PTHREAD_MUTEX_ROBUST_NP))) {
> > #ifdef PTHREAD_SETS_ERRNO
> > rv = errno;
> > -#endif
> > +#endif // DB
> > proc_mutex_proc_pthread_cleanup(new_mutex);
> > return rv;
> > }
> > +#endif
> > if ((rv = pthread_mutexattr_setprotocol(&mattr,
> > PTHREAD_PRIO_INHERIT))) {
> > #ifdef PTHREAD_SETS_ERRNO
> > rv = errno;
> > @@ -374,9 +381,15 @@
> > proc_mutex_proc_pthread_cleanup(new_mutex);
> > return rv;
> > }
> > + if ((rv =
> > pthread_mutex_destroy(new_mutex->pthread_interproc)))
> > { // DB
> > +#ifdef PTHREAD_SETS_ERRNO
> > + rv = errno;
> > #endif
> > + proc_mutex_proc_pthread_cleanup(new_mutex);
> > + return rv;
> > + }
> >
> > - if ((rv =
> > pthread_mutex_init(new_mutex->pthread_interproc,
> > &mattr))) {
> > + if ((rv =
> > pthread_mutex_init(new_mutex->pthread_interproc,
> > &mattr))) {
> > #ifdef PTHREAD_SETS_ERRNO
> > rv = errno;
> > #endif
> >
> > __________________________________________________
> > Do You Yahoo!?
> > LAUNCH - Your Yahoo! Music Experience
> > http://launch.yahoo.com
> >
>
Re: QNX 6.1a mod/peer review
Posted by Igor Kovalenko <Ig...@motorola.com>.
Was that applied or not? Does not seem to be in 2.0.39...
As for the content of patch, it appears to do 2 things: replace mmmap() of
/dev/zero with mmap() of a regular file (which is not the best idea) and
handle retuns of pthread_xxx() funcs differently (they return errno value
rather than set errno on QNX6). This all applies to
USE_PROC_PTHREAD_SERIALIZE code.
However, USE_PROC_PTHREAD_SERIALIZE is not defined on QNX6. The
USE_PTHREAD_SERIALIZE is defined, but useless since apache has no code for
it beyond handling -V option. USE_POSIXSEM_SERIALIZE is defined too and
SINGLE_LISTEN_UNSERIALIZED_ACCEPT is also defined.
Which means (as far as i can tell) USE_POSIXSEM_SERIALIZE will be used if
more than one Listen is present. That code looks somewhat strange to me too.
Is there a reason why named POSIX semaphores are used? Named ones add lot of
unnecessary trouble (there are even comments inline about joy of using them)
and on QNX they are also whole lot slower. Why not put unnamed ones into
anon shared memory? Is this code used by any other platform which does have
named POSIX sems but does not have anon shared memory? The time_sem.c does
not have code to test POSIXSEM_SERIALIZE performance. I did test FLOCK/FCNTL
and PTHREAD though and PTHREAD is 5-6 times faster on QNX6 (single CPU, 50
children, 10000 iterations).
So here is what needs to be done I think:
1. define USE_PROC_PTHREAD_SERIALIZE for QNX6
2. patch USE_PROC_PTHREAD_SERIALIZE to use mmap(MAP_ANON) is available and
fallback to mmap(/dev/zero) otherwise; also patch it to use QNX6 equivalent
of pthread_mutex_setconsistent_np().
3. patch for handling of pthread_xxx() returns.
4. handle scoreboard code to use mmap() in similar way to (2)
5. remove QNX6 from list of systems which don't have initgroups().
Are there upfront objections to such patch before I bother doing it? There
were objections when I tried to do (2) for apache1.3 - it was said to be
non-QNX-specific. My arguments (that it actually should be non-QNX-specific)
were never answered and patch was summarily ignored.
Regards,
-- igor
----- Original Message -----
From: "Davide Berti" <db...@yahoo.com>
To: <de...@httpd.apache.org>
Sent: Thursday, May 23, 2002 12:28 PM
Subject: QNX 6.1a mod/peer review
> Hello all
>
> httpd-2.0.36, QNX 6.1 RTOS
>
> I have made a modification to apache to get it to run
> on qnx. It kept hanging. I traced it down and made
> some changes to get it to run. I wanted to proof
> these changes with the apache community and get some
> feedback as to the longterm consequences/ side effects
> of these changes. I am attaching a diff of the
> changes that I made.
>
> I also noticed that SSL doesn't seem to work unless
> the -X flag is passed to httpd, any ideas.
>
> Thanks
> /Davide
>
> --- httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Mon Apr 8 23:56:56 2002
> +++ ../httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Wed May 8 16:04:51 2002
> @@ -318,7 +318,9 @@
> if (munmap((caddr_t)mutex->pthread_interproc,
> sizeof(pthread_mutex_t))){
> return errno;
> }
> - }
> + if(shm_unlink("/datapoints")) // DB
> + return errno;
> + }
> return APR_SUCCESS;
> }
>
> @@ -329,11 +331,15 @@
> int fd;
> pthread_mutexattr_t mattr;
>
> - fd = open("/dev/zero", O_RDWR);
> - if (fd < 0) {
> - return errno;
> - }
> + fd=shm_open("/datapoints",O_RDWR|O_CREAT,0777);
> file://DB
> + if (fd < 0)
> + return errno;
>
> + if(ftruncate(fd,sizeof(pthread_mutex_t))==-1) file://DB
> + return errno;
> +
> +
> +
> new_mutex->pthread_interproc = (pthread_mutex_t
> *)mmap(
> (caddr_t) 0,
>
> sizeof(pthread_mutex_t),
> @@ -363,10 +369,11 @@
>
> PTHREAD_MUTEX_ROBUST_NP))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> -#endif
> +#endif // DB
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> +#endif
> if ((rv = pthread_mutexattr_setprotocol(&mattr,
> PTHREAD_PRIO_INHERIT))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> @@ -374,9 +381,15 @@
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> + if ((rv =
> pthread_mutex_destroy(new_mutex->pthread_interproc)))
> { // DB
> +#ifdef PTHREAD_SETS_ERRNO
> + rv = errno;
> #endif
> + proc_mutex_proc_pthread_cleanup(new_mutex);
> + return rv;
> + }
>
> - if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> + if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> #endif
>
> __________________________________________________
> Do You Yahoo!?
> LAUNCH - Your Yahoo! Music Experience
> http://launch.yahoo.com
>
Re: QNX 6.1a mod/peer review
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 23, 2002 at 05:36:26PM -0500, Igor Kovalenko wrote:
> I had submitted patch foir QNX6 twice in the past. There was no ACK and
> generally it was ignored. The patch was used for about half a year on public
> site without any problems.
You should keep resubmitting (once a week or so) until someone
says something (positive or negative).
That is our policy. We have so many different people reading this
list from week-to-week that you'll get a different audience each
week. -- justin
Re: QNX 6.1a mod/peer review
Posted by Igor Kovalenko <Ig...@motorola.com>.
I had submitted patch foir QNX6 twice in the past. There was no ACK and
generally it was ignored. The patch was used for about half a year on public
site without any problems.
-- Igor
----- Original Message -----
From: "Davide Berti" <db...@yahoo.com>
To: <de...@httpd.apache.org>
Sent: Thursday, May 23, 2002 12:28 PM
Subject: QNX 6.1a mod/peer review
> Hello all
>
> httpd-2.0.36, QNX 6.1 RTOS
>
> I have made a modification to apache to get it to run
> on qnx. It kept hanging. I traced it down and made
> some changes to get it to run. I wanted to proof
> these changes with the apache community and get some
> feedback as to the longterm consequences/ side effects
> of these changes. I am attaching a diff of the
> changes that I made.
>
> I also noticed that SSL doesn't seem to work unless
> the -X flag is passed to httpd, any ideas.
>
> Thanks
> /Davide
>
> --- httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Mon Apr 8 23:56:56 2002
> +++ ../httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Wed May 8 16:04:51 2002
> @@ -318,7 +318,9 @@
> if (munmap((caddr_t)mutex->pthread_interproc,
> sizeof(pthread_mutex_t))){
> return errno;
> }
> - }
> + if(shm_unlink("/datapoints")) // DB
> + return errno;
> + }
> return APR_SUCCESS;
> }
>
> @@ -329,11 +331,15 @@
> int fd;
> pthread_mutexattr_t mattr;
>
> - fd = open("/dev/zero", O_RDWR);
> - if (fd < 0) {
> - return errno;
> - }
> + fd=shm_open("/datapoints",O_RDWR|O_CREAT,0777);
> file://DB
> + if (fd < 0)
> + return errno;
>
> + if(ftruncate(fd,sizeof(pthread_mutex_t))==-1) file://DB
> + return errno;
> +
> +
> +
> new_mutex->pthread_interproc = (pthread_mutex_t
> *)mmap(
> (caddr_t) 0,
>
> sizeof(pthread_mutex_t),
> @@ -363,10 +369,11 @@
>
> PTHREAD_MUTEX_ROBUST_NP))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> -#endif
> +#endif // DB
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> +#endif
> if ((rv = pthread_mutexattr_setprotocol(&mattr,
> PTHREAD_PRIO_INHERIT))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> @@ -374,9 +381,15 @@
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> + if ((rv =
> pthread_mutex_destroy(new_mutex->pthread_interproc)))
> { // DB
> +#ifdef PTHREAD_SETS_ERRNO
> + rv = errno;
> #endif
> + proc_mutex_proc_pthread_cleanup(new_mutex);
> + return rv;
> + }
>
> - if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> + if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> #endif
>
> __________________________________________________
> Do You Yahoo!?
> LAUNCH - Your Yahoo! Music Experience
> http://launch.yahoo.com
>
Re: QNX 6.1a mod/peer review
Posted by Justin Erenkrantz <je...@apache.org>.
[ Moving to dev@apr which is the right list for this. ]
Comments inline.
On Thu, May 23, 2002 at 10:28:08AM -0700, Davide Berti wrote:
> --- httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Mon Apr 8 23:56:56 2002
> +++ ../httpd-2.0.36/srclib/apr/locks/unix/proc_mutex.c
> Wed May 8 16:04:51 2002
> @@ -318,7 +318,9 @@
> if (munmap((caddr_t)mutex->pthread_interproc,
> sizeof(pthread_mutex_t))){
> return errno;
> }
> - }
> + if(shm_unlink("/datapoints")) // DB
> + return errno;
> + }
Stylistic concerns:
Lose the // DB comments and the tabs. We only use spaces (4 space
indentions). You should read our style guide:
http://httpd.apache.org/dev/styleguide.html
Does QNX not have /dev/zero? What was the problem you were seeing?
What is /datapoints? Obviously, your patch breaks other platforms.
If you wish us to integrate it into our sources, you need to make
it so that it doesn't break other platforms. Since it sounds
like /dev/zero doesn't exist, you need a shared data source -
is /datapoints a QNX convention or something you came up with?
If this isn't a "special file", you'll have a few problems:
1) How will other users (non-root) be able to access this file?
(ISTR that QNX doesn't have users, so that may be why.)
2) If there is a "dead" process that died due to a segfault,
this file won't be removed, so you'll have to remove this file
yourself. (This is why SysV mutexes are so problematic.)
> @@ -329,11 +331,15 @@
> int fd;
> pthread_mutexattr_t mattr;
>
> - fd = open("/dev/zero", O_RDWR);
> - if (fd < 0) {
> - return errno;
> - }
> + fd=shm_open("/datapoints",O_RDWR|O_CREAT,0777);
> //DB
> + if (fd < 0)
> + return errno;
>
> + if(ftruncate(fd,sizeof(pthread_mutex_t))==-1) //DB
> + return errno;
> +
> +
> +
Lose the extra blank lines. You also removed the {} on one-line
if statements which is our style.
Any particular reason why the fd must be truncated? We only use
/dev/zero because it's a cheap place to get file-backed storage - we
don't rely upon the fact that it is zeroed out. So, you should be
able to get away without this.
> @@ -363,10 +369,11 @@
>
> PTHREAD_MUTEX_ROBUST_NP))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> -#endif
> +#endif // DB
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> +#endif
> if ((rv = pthread_mutexattr_setprotocol(&mattr,
> PTHREAD_PRIO_INHERIT))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
You added an #endif - why?
> @@ -374,9 +381,15 @@
> proc_mutex_proc_pthread_cleanup(new_mutex);
> return rv;
> }
> + if ((rv =
> pthread_mutex_destroy(new_mutex->pthread_interproc)))
> { // DB
> +#ifdef PTHREAD_SETS_ERRNO
> + rv = errno;
> #endif
> + proc_mutex_proc_pthread_cleanup(new_mutex);
> + return rv;
> + }
>
> - if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> + if ((rv =
> pthread_mutex_init(new_mutex->pthread_interproc,
> &mattr))) {
> #ifdef PTHREAD_SETS_ERRNO
> rv = errno;
> #endif
Why call destroy before calling init? (Lose your tabs->space
conversion, too.)
All in all, this might not be a bad road to proceed down so that
on platforms without /dev/zero, we could use pthread cross-process
mutexes, but I doubt it'll be easy.
I know our autoconf test won't recognize pthread proc mutex in
this case since the test checks for /dev/zero. So, you'll have
to modify that as weel to get this to be included. -- justin