You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by "MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1)" <ma...@hp.com> on 2002/12/11 06:40:09 UTC
[PATCH] Use mutex locks in mod_specweb99.c
I started seeing the following errors in the specweb99 run output, when I
use mod_specweb99.c with Apache 2.0.43 and worker MPM. The following patch
seems to get rid of the problem. If you're thinking that it may degrade the
response - I did not find much difference though.
Can somebody please evaluate and let me know if it's okay ?.
----------------------------------------------------------------
ERRORS FOUND
Iteration 3: out of order entry in log
Errorseen: 0, lastseen= 3694
Count: 3694, num= 6
Iteration 3: Number of POST requests in log inconsistant
Reference_count = 238213, real_count = 239179
-----------------------------------------------------------------
Thanks
-Madhu
Index: mod_specweb99.c
===================================================================
RCS file:
/home/cvspublic/httpd-test/specweb99/specweb99-2.0/mod_specweb99.c,v
retrieving revision 1.16
diff -u -r1.16 mod_specweb99.c
--- mod_specweb99.c 31 Oct 2002 19:39:04 -0000 1.16
+++ mod_specweb99.c 11 Dec 2002 05:30:12 -0000
@@ -83,6 +83,10 @@
#include <unistd.h>
#endif
+#if APR_HAS_THREADS
+apr_thread_mutex_t *log_mutex;
+#endif
+
/* Note: version must be of the x.yy type - as it is
* send over the http protocol wire; where x and y
* are single 0-9 ascii digits :-). The name should
@@ -600,6 +604,10 @@
if (apr_pool_create(&(_my->cad_pool), p) != APR_SUCCESS)
exit(APEXIT_CHILDFATAL);
+#if APR_HAS_THREADS
+ apr_thread_mutex_create(&log_mutex, APR_THREAD_MUTEX_DEFAULT, p);
+#endif
+
if (s->next) {
fprintf(stderr,
"WARNING- this specweb module currently does not support
vhosts/services\n"
@@ -1175,8 +1183,15 @@
return HTTP_INTERNAL_SERVER_ERROR;
}
- if ((rv = _wlock(r->server, r, f, _my->log_path)) != APR_SUCCESS)
+#if APR_HAS_THREADS
+ apr_thread_mutex_lock(log_mutex);
+#endif
+ if ((rv = _wlock(r->server, r, f, _my->log_path)) != APR_SUCCESS) {
+#if APR_HAS_THREADS
+ apr_thread_mutex_unlock(log_mutex);
+#endif
returnHTMLPageWithMessage(r, "Failed to lock post.log file");
+ }
else {
char *msg =
_log_and_write(r, f, filename, urlroot, dirnum, classnum, filenum,
@@ -1184,6 +1199,9 @@
if (msg) {
rv = APR_OS_START_USEERR;
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, msg);
+#if APR_HAS_THREADS
+ apr_thread_mutex_unlock(log_mutex);
+#endif
returnHTMLPageWithMessage(r, msg);
}
}
@@ -1193,10 +1211,16 @@
rv = rv2;
ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,
"Failed to unlock %s", filename ? filename : "");
+#if APR_HAS_THREADS
+ apr_thread_mutex_unlock(log_mutex);
+#endif
returnHTMLPageWithMessage(r, "Failed to lock unpost.log file");
}
}
+#if APR_HAS_THREADS
+ apr_thread_mutex_unlock(log_mutex);
+#endif
apr_file_close(f);
if (rv != APR_SUCCESS)
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Greg Ames <gr...@apache.org>.
MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) wrote:
> I started seeing the following errors in the specweb99 run output, when I
> use mod_specweb99.c with Apache 2.0.43 and worker MPM. The following patch
> seems to get rid of the problem. If you're thinking that it may degrade the
> response - I did not find much difference though.
>
> Can somebody please evaluate and let me know if it's okay ?.
[...]
> +#if APR_HAS_THREADS
> + apr_thread_mutex_lock(log_mutex);
> +#endif
from my Linux srclib/apr/include/apr.h, with Apache configured to run the
prefork MPM:
#define APR_HAS_THREADS 1
...so I would see some degradation running prefork without getting any benefit
from the additional mutexing.
I believe APR has global locks available on every platform we support, where
global means that the lock mechanism serializes threads within a process as well
as cross-process. If that's true, I would prefer that we start by switching to
that global lock mechanism to replace the apr_file_lock. If on some platform
APR's global lock doesn't perform adequately, we can deal with that on a
case-by-case basis.
A possible complication: read/write lock support. How important is that? I
don't know how frequently SPECWeb99 reads the post log. If it doesn't happen
often compared to the POST URLs which would need a write lock, maybe we can get
by with just a write lock.
Greg
p.s. at the moment, by far the heaviest CPU usage in my Linux SPECWeb99 config
is in csum_partial_copy_generic, a kernel function that calculates TCP checksums
and copies data to/from userspace. I'd like to get a NIC with hardware checksum
support, which should give me some relief. But in the mean time, I'm not doing
much benchmarking because it's hard to see meaningful differences when I make
changes.
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Aaron Bannert <aa...@clove.org>.
On Friday, December 13, 2002, at 10:05 AM, <rb...@apache.org> wrote:
>> They are in apr_private.h.
>
> Yes, but they are also in apr.h.
>
> pigmy:~/apr/include - 9:57AM% grep "_IS_GLOBAL" apr.h
> #define APR_PROCESS_LOCK_IS_GLOBAL 0
> #define APR_PROC_MUTEX_IS_GLOBAL 0
> pigmy:~/apr/include - 9:57AM% cd arch/unix
> pigmy:~/apr/include/arch/unix - 9:57AM% grep "_IS_GLOBAL"
> apr_private.h
> /* #undef POSIXSEM_IS_GLOBAL */
> /* #undef SYSVSEM_IS_GLOBAL */
> /* #undef FCNTL_IS_GLOBAL */
> /* #undef FLOCK_IS_GLOBAL */
> pigmy:~/apr/include/arch/unix - 9:57AM%
>
>
> So, why are they in apr.h at all?
Because they are used in public header files, and we don't want
to pollute the public header files with symbols from the private
headers.
aaron@gandalf% grep -lr _IS_GLOBAL . | grep \.h\$
./acconfig.h
./include/apr.h
./include/apr_global_mutex.h
./include/apr_portable.h
./include/arch/unix/apr_private.h
./include/arch/unix/proc_mutex.h
-aaron
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by rb...@apache.org.
On Fri, 13 Dec 2002, Aaron Bannert wrote:
>
> On Friday, December 13, 2002, at 09:06 AM, <rb...@apache.org> wrote:
>
> > On Fri, 13 Dec 2002, Aaron Bannert wrote:
> >
> >>
> >> On Thursday, December 12, 2002, at 12:34 PM, Greg Ames wrote:
> >>> I dug into APR locks a little bit. The apr_global_mutex_* functions
> >>> turn into two separate syscalls, with #if APR_HAS_THREADS around the
> >>> thread mutexing. So unfortunately they wouldn't save us any syscalls
> >>> :-( :-( But they might save a little bit of function call overhead.
> >>>
> >>> Another interesting place to look is in
> >>> srclib/apr/include/arch/unix/apr_private.h .
> >>> There are several xxxx_IS_GLOBAL symbols for various serialization
> >>> mechanisms. On my Linux box, all of them are #undef'ed and commented
> >>> out, including fcntl and flock which are the two choices for
> >>> apr_file_lock. Madhu, could you take a look there and see what
> >>> you've
> >>> got?
> >>
> >> You really shouldn't use those xxxx_IS_GLOBAL symbols for anything
> >> outside of APR.
> >> They are there for platforms like s390 where some of the file-based
> >> locks do actually
> >> serialize multiple threads within multiple processes.
> >
> > Um, then why in the world are those symbols in a public header file?
> > They
> > are only ever used inside of APR, so they should exist in
> > apr_private.h,
> > not apr.h.
>
> They are in apr_private.h.
Yes, but they are also in apr.h.
pigmy:~/apr/include - 9:57AM% grep "_IS_GLOBAL" apr.h
#define APR_PROCESS_LOCK_IS_GLOBAL 0
#define APR_PROC_MUTEX_IS_GLOBAL 0
pigmy:~/apr/include - 9:57AM% cd arch/unix
pigmy:~/apr/include/arch/unix - 9:57AM% grep "_IS_GLOBAL" apr_private.h
/* #undef POSIXSEM_IS_GLOBAL */
/* #undef SYSVSEM_IS_GLOBAL */
/* #undef FCNTL_IS_GLOBAL */
/* #undef FLOCK_IS_GLOBAL */
pigmy:~/apr/include/arch/unix - 9:57AM%
So, why are they in apr.h at all?
Ryan
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Aaron Bannert <aa...@clove.org>.
On Friday, December 13, 2002, at 09:06 AM, <rb...@apache.org> wrote:
> On Fri, 13 Dec 2002, Aaron Bannert wrote:
>
>>
>> On Thursday, December 12, 2002, at 12:34 PM, Greg Ames wrote:
>>> I dug into APR locks a little bit. The apr_global_mutex_* functions
>>> turn into two separate syscalls, with #if APR_HAS_THREADS around the
>>> thread mutexing. So unfortunately they wouldn't save us any syscalls
>>> :-( :-( But they might save a little bit of function call overhead.
>>>
>>> Another interesting place to look is in
>>> srclib/apr/include/arch/unix/apr_private.h .
>>> There are several xxxx_IS_GLOBAL symbols for various serialization
>>> mechanisms. On my Linux box, all of them are #undef'ed and commented
>>> out, including fcntl and flock which are the two choices for
>>> apr_file_lock. Madhu, could you take a look there and see what
>>> you've
>>> got?
>>
>> You really shouldn't use those xxxx_IS_GLOBAL symbols for anything
>> outside of APR.
>> They are there for platforms like s390 where some of the file-based
>> locks do actually
>> serialize multiple threads within multiple processes.
>
> Um, then why in the world are those symbols in a public header file?
> They
> are only ever used inside of APR, so they should exist in
> apr_private.h,
> not apr.h.
They are in apr_private.h.
-aaron
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by rb...@apache.org.
On Fri, 13 Dec 2002, Aaron Bannert wrote:
>
> On Thursday, December 12, 2002, at 12:34 PM, Greg Ames wrote:
> > I dug into APR locks a little bit. The apr_global_mutex_* functions
> > turn into two separate syscalls, with #if APR_HAS_THREADS around the
> > thread mutexing. So unfortunately they wouldn't save us any syscalls
> > :-( :-( But they might save a little bit of function call overhead.
> >
> > Another interesting place to look is in
> > srclib/apr/include/arch/unix/apr_private.h .
> > There are several xxxx_IS_GLOBAL symbols for various serialization
> > mechanisms. On my Linux box, all of them are #undef'ed and commented
> > out, including fcntl and flock which are the two choices for
> > apr_file_lock. Madhu, could you take a look there and see what you've
> > got?
>
> You really shouldn't use those xxxx_IS_GLOBAL symbols for anything
> outside of APR.
> They are there for platforms like s390 where some of the file-based
> locks do actually
> serialize multiple threads within multiple processes.
Um, then why in the world are those symbols in a public header file? They
are only ever used inside of APR, so they should exist in apr_private.h,
not apr.h.
Ryan
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Sander Temme <sc...@covalent.net>.
> If you're worried about the overhead of calling pthread_lock and
> pthread_unlock under the covers when it's not needed (like on the prefork MPM)
> then I would say that's not a big problem. Locking mutexes that are not
> contended for can be quite cheap, and often those calls do not even translate
> into syscalls.
What we're pondering here is locking a logfile in mod_specweb99. During a
SPECWeb99 run, mutexes are not cheap and there is a lot of camping out going
on.
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Aaron Bannert <aa...@clove.org>.
On Thursday, December 12, 2002, at 12:34 PM, Greg Ames wrote:
> I dug into APR locks a little bit. The apr_global_mutex_* functions
> turn into two separate syscalls, with #if APR_HAS_THREADS around the
> thread mutexing. So unfortunately they wouldn't save us any syscalls
> :-( :-( But they might save a little bit of function call overhead.
>
> Another interesting place to look is in
> srclib/apr/include/arch/unix/apr_private.h .
> There are several xxxx_IS_GLOBAL symbols for various serialization
> mechanisms. On my Linux box, all of them are #undef'ed and commented
> out, including fcntl and flock which are the two choices for
> apr_file_lock. Madhu, could you take a look there and see what you've
> got?
You really shouldn't use those xxxx_IS_GLOBAL symbols for anything
outside of APR.
They are there for platforms like s390 where some of the file-based
locks do actually
serialize multiple threads within multiple processes.
If you're worried about the overhead of calling pthread_lock and
pthread_unlock
under the covers when it's not needed (like on the prefork MPM) then I
would
say that's not a big problem. Locking mutexes that are not contended
for can
be quite cheap, and often those calls do not even translate into
syscalls.
-aaron
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Greg Ames <gr...@apache.org>.
Sander Temme wrote:
>>I started seeing the following errors in the specweb99 run output, when I
>>use mod_specweb99.c with Apache 2.0.43 and worker MPM. The following patch
>>seems to get rid of the problem. If you're thinking that it may degrade the
>>response - I did not find much difference though.
>>
>>Can somebody please evaluate and let me know if it's okay ?.
>
>
> Ha! I have seen this too but have had no time to even think about working on
> it.
>
> I have one question. Your patch mutexes out the acquisition of the file
> lock. Do these thread mutexes apply only within the process, or across
> processes as well? In the latter case, we could do away with the flock
> entirely if we're in a multithreaded environment. In that case the #ifs
> would move to the _dolock function and we'd have an _unlock function with
> its own #ifs.
I dug into APR locks a little bit. The apr_global_mutex_* functions turn into
two separate syscalls, with #if APR_HAS_THREADS around the thread mutexing. So
unfortunately they wouldn't save us any syscalls :-( :-( But they might save a
little bit of function call overhead.
Another interesting place to look is in srclib/apr/include/arch/unix/apr_private.h .
There are several xxxx_IS_GLOBAL symbols for various serialization mechanisms.
On my Linux box, all of them are #undef'ed and commented out, including fcntl
and flock which are the two choices for apr_file_lock. Madhu, could you take a
look there and see what you've got?
Thanks,
Greg
Re: [PATCH] Use mutex locks in mod_specweb99.c
Posted by Sander Temme <sc...@covalent.net>.
> I started seeing the following errors in the specweb99 run output, when I
> use mod_specweb99.c with Apache 2.0.43 and worker MPM. The following patch
> seems to get rid of the problem. If you're thinking that it may degrade the
> response - I did not find much difference though.
>
> Can somebody please evaluate and let me know if it's okay ?.
Ha! I have seen this too but have had no time to even think about working on
it.
I have one question. Your patch mutexes out the acquisition of the file
lock. Do these thread mutexes apply only within the process, or across
processes as well? In the latter case, we could do away with the flock
entirely if we're in a multithreaded environment. In that case the #ifs
would move to the _dolock function and we'd have an _unlock function with
its own #ifs.
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================