* [PATCH] Don't use catomic_* in __rtld_mrlock*
@ 2006-10-17 14:00 Jakub Jelinek
2006-10-18 19:21 ` Ulrich Drepper
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2006-10-17 14:00 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Glibc hackers
Hi!
With the latest switch to catomic_* in __rtld_mrlock* and related code,
I believe we are unnecessarily inefficient.
E.g. in
__rtld_mrlock_lock (l->l_scoperec_lock);
scoperec = l->l_scoperec;
catomic_increment (&scoperec->nusers);
__rtld_mrlock_unlock (l->l_scoperec_lock);
we at least 3 times read self->header.self and branch depending if
it is zero or not. But we know that a valid program can't
call pthread_create from within dl_lookup_symbol_x or signal handlers,
therefore just one SINGLE_THREAD_P check should be enough.
The following patch implements it, in the most performance critical
places (dl-runtime.c and dl-sym.c) it means fewer executed instructions
both in the single thread and multiple thread cases (in dl-sym.c even
far faster for single thread case) and shorter code.
2006-10-17 Jakub Jelinek <jakub@redhat.com>
* elf/dl-runtime.c: Include sysdep-cancel.h.
(_dl_fixup, _dl_profile_fixup): Use __rtld_mrlock_* and
scoperec->nusers only if !SINGLE_THREAD_P. Use atomic_*
instead of catomic_* macros.
* elf/dl-sym.c: Include sysdep-cancel.h.
(do_sym): Use __rtld_mrlock_* and scoperec->nusers only
if !SINGLE_THREAD_P. Use atomic_* instead of catomic_* macros.
* elf/dl-close.c: Include sysdep-cancel.h.
(_dl_close): Use __rtld_mrlock_* and scoperec->nusers only
if !SINGLE_THREAD_P. Use atomic_* instead of catomic_* macros.
* elf/dl-open.c: Include sysdep-cancel.h.
(dl_open_worker): Use __rtld_mrlock_* and scoperec->nusers only
if !SINGLE_THREAD_P. Use atomic_* instead of catomic_* macros.
nptl/
* sysdeps/unix/sysv/linux/rtld-lowlevel.h (__rtld_mrlock_lock,
__rtld_mrlock_unlock, __rtld_mrlock_change, __rtld_mrlock_done): Use
atomic_* instead of catomic_* macros.
--- libc/elf/dl-runtime.c.jj 2006-10-17 00:12:31.000000000 +0200
+++ libc/elf/dl-runtime.c 2006-10-17 14:55:35.000000000 +0200
@@ -24,6 +24,7 @@
#include <unistd.h>
#include <sys/param.h>
#include <ldsodefs.h>
+#include <sysdep-cancel.h>
#include "dynamic-link.h"
#if (!defined ELF_MACHINE_NO_RELA && !defined ELF_MACHINE_PLT_REL) \
@@ -93,11 +94,11 @@ _dl_fixup (
}
struct r_scoperec *scoperec = l->l_scoperec;
- if (l->l_type == lt_loaded)
+ if (l->l_type == lt_loaded && !SINGLE_THREAD_P)
{
__rtld_mrlock_lock (l->l_scoperec_lock);
scoperec = l->l_scoperec;
- catomic_increment (&scoperec->nusers);
+ atomic_increment (&scoperec->nusers);
__rtld_mrlock_unlock (l->l_scoperec_lock);
}
@@ -106,8 +107,8 @@ _dl_fixup (
ELF_RTYPE_CLASS_PLT,
DL_LOOKUP_ADD_DEPENDENCY, NULL);
- if (l->l_type == lt_loaded
- && catomic_decrement_val (&scoperec->nusers) == 0
+ if (l->l_type == lt_loaded && !SINGLE_THREAD_P
+ && atomic_decrement_val (&scoperec->nusers) == 0
&& __builtin_expect (scoperec->remove_after_use, 0))
{
if (scoperec->notify)
@@ -195,11 +196,11 @@ _dl_profile_fixup (
}
struct r_scoperec *scoperec = l->l_scoperec;
- if (l->l_type == lt_loaded)
+ if (l->l_type == lt_loaded && !SINGLE_THREAD_P)
{
__rtld_mrlock_lock (l->l_scoperec_lock);
scoperec = l->l_scoperec;
- catomic_increment (&scoperec->nusers);
+ atomic_increment (&scoperec->nusers);
__rtld_mrlock_unlock (l->l_scoperec_lock);
}
@@ -208,8 +209,8 @@ _dl_profile_fixup (
ELF_RTYPE_CLASS_PLT,
DL_LOOKUP_ADD_DEPENDENCY, NULL);
- if (l->l_type == lt_loaded
- && catomic_decrement_val (&scoperec->nusers) == 0
+ if (l->l_type == lt_loaded && !SINGLE_THREAD_P
+ && atomic_decrement_val (&scoperec->nusers) == 0
&& __builtin_expect (scoperec->remove_after_use, 0))
{
if (scoperec->notify)
--- libc/elf/dl-sym.c.jj 2006-10-17 00:12:31.000000000 +0200
+++ libc/elf/dl-sym.c 2006-10-17 14:56:59.000000000 +0200
@@ -25,6 +25,7 @@
#include <dlfcn.h>
#include <ldsodefs.h>
#include <dl-hash.h>
+#include <sysdep-cancel.h>
#ifdef USE_TLS
# include <dl-tls.h>
#endif
@@ -115,7 +116,7 @@ do_sym (void *handle, const char *name,
the initial binary. And then the more complex part
where the object is dynamically loaded and the scope
array can change. */
- if (match->l_type != lt_loaded)
+ if (match->l_type != lt_loaded || SINGLE_THREAD_P)
result = GLRO(dl_lookup_symbol_x) (name, match, &ref,
match->l_scoperec->scope, vers, 0,
flags | DL_LOOKUP_ADD_DEPENDENCY,
@@ -124,7 +125,7 @@ do_sym (void *handle, const char *name,
{
__rtld_mrlock_lock (match->l_scoperec_lock);
struct r_scoperec *scoperec = match->l_scoperec;
- catomic_increment (&scoperec->nusers);
+ atomic_increment (&scoperec->nusers);
__rtld_mrlock_unlock (match->l_scoperec_lock);
struct call_dl_lookup_args args;
@@ -141,7 +142,7 @@ do_sym (void *handle, const char *name,
int err = GLRO(dl_catch_error) (&objname, &errstring, &malloced,
call_dl_lookup, &args);
- if (catomic_decrement_val (&scoperec->nusers) == 0
+ if (atomic_decrement_val (&scoperec->nusers) == 0
&& __builtin_expect (scoperec->remove_after_use, 0))
{
if (scoperec->notify)
--- libc/elf/dl-close.c.jj 2006-10-17 00:12:31.000000000 +0200
+++ libc/elf/dl-close.c 2006-10-17 15:01:18.000000000 +0200
@@ -30,6 +30,7 @@
#include <ldsodefs.h>
#include <sys/types.h>
#include <sys/mman.h>
+#include <sysdep-cancel.h>
/* Type of the constructor functions. */
@@ -419,16 +420,21 @@ _dl_close (void *_map)
struct r_scoperec *old = imap->l_scoperec;
- __rtld_mrlock_change (imap->l_scoperec_lock);
- imap->l_scoperec = newp;
- __rtld_mrlock_done (imap->l_scoperec_lock);
-
- if (catomic_increment_val (&old->nusers) != 1)
+ if (SINGLE_THREAD_P)
+ imap->l_scoperec = newp;
+ else
{
- old->remove_after_use = true;
- old->notify = true;
- if (catomic_decrement_val (&old->nusers) != 0)
- __rtld_waitzero (old->nusers);
+ __rtld_mrlock_change (imap->l_scoperec_lock);
+ imap->l_scoperec = newp;
+ __rtld_mrlock_done (imap->l_scoperec_lock);
+
+ if (atomic_increment_val (&old->nusers) != 1)
+ {
+ old->remove_after_use = true;
+ old->notify = true;
+ if (atomic_decrement_val (&old->nusers) != 0)
+ __rtld_waitzero (old->nusers);
+ }
}
/* No user anymore, we can free it now. */
--- libc/elf/dl-open.c.jj 2006-10-17 00:12:31.000000000 +0200
+++ libc/elf/dl-open.c 2006-10-17 15:30:16.000000000 +0200
@@ -31,6 +31,7 @@
#include <ldsodefs.h>
#include <bp-sym.h>
#include <caller.h>
+#include <sysdep-cancel.h>
#include <dl-dst.h>
@@ -423,15 +424,20 @@ dl_open_worker (void *a)
if (old == &imap->l_scoperec_mem)
imap->l_scoperec = newp;
+ else if (SINGLE_THREAD_P)
+ {
+ imap->l_scoperec = newp;
+ free (old);
+ }
else
{
__rtld_mrlock_change (imap->l_scoperec_lock);
imap->l_scoperec = newp;
__rtld_mrlock_done (imap->l_scoperec_lock);
- catomic_increment (&old->nusers);
+ atomic_increment (&old->nusers);
old->remove_after_use = true;
- if (catomic_decrement_val (&old->nusers) == 0)
+ if (atomic_decrement_val (&old->nusers) == 0)
/* No user, we can free it here and now. */
free (old);
}
--- libc/nptl/sysdeps/unix/sysv/linux/rtld-lowlevel.h.jj 2006-10-11 10:59:28.000000000 +0200
+++ libc/nptl/sysdeps/unix/sysv/linux/rtld-lowlevel.h 2006-10-17 14:52:22.000000000 +0200
@@ -62,9 +62,9 @@ typedef int __rtld_mrlock_t;
{ \
int newval = ((oldval & __RTLD_MRLOCK_RBITS) \
+ __RTLD_MRLOCK_INC); \
- int ret = catomic_compare_and_exchange_val_acq (&(lock), \
- newval, \
- oldval); \
+ int ret = atomic_compare_and_exchange_val_acq (&(lock), \
+ newval, \
+ oldval); \
if (__builtin_expect (ret == oldval, 1)) \
goto out; \
} \
@@ -72,7 +72,7 @@ typedef int __rtld_mrlock_t;
} \
if ((oldval & __RTLD_MRLOCK_RWAIT) == 0) \
{ \
- catomic_or (&(lock), __RTLD_MRLOCK_RWAIT); \
+ atomic_or (&(lock), __RTLD_MRLOCK_RWAIT); \
oldval |= __RTLD_MRLOCK_RWAIT; \
} \
lll_futex_wait (lock, oldval); \
@@ -83,7 +83,7 @@ typedef int __rtld_mrlock_t;
#define __rtld_mrlock_unlock(lock) \
do { \
- int oldval = catomic_exchange_and_add (&(lock), -__RTLD_MRLOCK_INC); \
+ int oldval = atomic_exchange_and_add (&(lock), -__RTLD_MRLOCK_INC); \
if (__builtin_expect ((oldval \
& (__RTLD_MRLOCK_RBITS | __RTLD_MRLOCK_WWAIT)) \
== (__RTLD_MRLOCK_INC | __RTLD_MRLOCK_WWAIT), 0)) \
@@ -103,11 +103,11 @@ typedef int __rtld_mrlock_t;
for (int tries = 0; tries < __RTLD_MRLOCK_TRIES; ++tries) \
{ \
oldval = lock; \
- while (__builtin_expect ((oldval & __RTLD_MRLOCK_RBITS) == 0, 1))\
+ while (__builtin_expect ((oldval & __RTLD_MRLOCK_RBITS) == 0, 1)) \
{ \
int newval = ((oldval & __RTLD_MRLOCK_RWAIT) \
+ __RTLD_MRLOCK_WRITER); \
- int ret = catomic_compare_and_exchange_val_acq (&(lock), \
+ int ret = atomic_compare_and_exchange_val_acq (&(lock), \
newval, \
oldval); \
if (__builtin_expect (ret == oldval, 1)) \
@@ -115,7 +115,7 @@ typedef int __rtld_mrlock_t;
} \
atomic_delay (); \
} \
- catomic_or (&(lock), __RTLD_MRLOCK_WWAIT); \
+ atomic_or (&(lock), __RTLD_MRLOCK_WWAIT); \
oldval |= __RTLD_MRLOCK_WWAIT; \
lll_futex_wait (lock, oldval); \
} \
@@ -125,7 +125,7 @@ typedef int __rtld_mrlock_t;
#define __rtld_mrlock_done(lock) \
do { \
- int oldval = catomic_exchange_and_add (&(lock), -__RTLD_MRLOCK_WRITER); \
+ int oldval = atomic_exchange_and_add (&(lock), -__RTLD_MRLOCK_WRITER); \
if (__builtin_expect ((oldval & __RTLD_MRLOCK_RWAIT) != 0, 0)) \
lll_futex_wake (&(lock), 0x7fffffff); \
} while (0)
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Don't use catomic_* in __rtld_mrlock*
2006-10-17 14:00 [PATCH] Don't use catomic_* in __rtld_mrlock* Jakub Jelinek
@ 2006-10-18 19:21 ` Ulrich Drepper
0 siblings, 0 replies; 2+ messages in thread
From: Ulrich Drepper @ 2006-10-18 19:21 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Glibc hackers
OK, but we must make sure we don't introduce any problem by having hte
lookup functions call into the audit interfaces. Ever.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA â–
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-10-18 19:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-17 14:00 [PATCH] Don't use catomic_* in __rtld_mrlock* Jakub Jelinek
2006-10-18 19:21 ` Ulrich Drepper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).