This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [PATCH 1/5][v2][BZ #15022] Avoid repeated calls to DL_STATIC_INIT
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, GNU C Library <libc-alpha at sourceware dot org>, <libc-ports at sourceware dot org>
- Date: Thu, 27 Jun 2013 11:52:19 +0100
- Subject: Re: [PATCH 1/5][v2][BZ #15022] Avoid repeated calls to DL_STATIC_INIT
- References: <alpine dot DEB dot 1 dot 10 dot 1301152056590 dot 4834 at tp dot orcam dot me dot uk> <20130116215545 dot 7A37A2C0B0 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1301240655220 dot 4834 at tp dot orcam dot me dot uk> <20130531200059 dot C94C02C077 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1306140202520 dot 16287 at tp dot orcam dot me dot uk> <20130619233103 dot A913F2C0A6 at topped-with-meat dot com> <CAMe9rOpR6OGW6CfrG5NN2HPM_=tW-1om4Y_dFVXmUk2h5xg6sQ at mail dot gmail dot com> <20130620204740 dot 69A5E2C135 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1306211348160 dot 16287 at tp dot orcam dot me dot uk> <20130621181119 dot DACD52C09F at topped-with-meat dot com>
On Fri, 21 Jun 2013, Roland McGrath wrote:
> I concur with your analysis. (I was considering the relationship of the
> two locks when responding to HJ earlier, but failed to be clear about it.)
> I think it's wise to make the removal of the extraneous lock a separate
> follow-up change, just for paranoia's sake (i.e. ease of reversion if
> needed for some unforeseen reason).
Thanks for confirming. I have applied this change now, and the change
below to remove the lock (no regressions in testing).
> The issue of the internal lock held while calling user initializers is
> entirely separate. It has concerned me before when I've noticed it in
> passing, but I never dawdled long enough to bring it up for discussion. I
> think you should file a bug about that one and we'll consider it separately
> later. But that will clearly have to be after the impending freeze.
BZ #15686 now.
2013-06-27 Maciej W. Rozycki <macro@codesourcery.com>
ports/ChangeLog.ia64
* sysdeps/unix/sysv/linux/ia64/dl-static.c: Do not include
<bits/libc-lock.h>.
(_dl_static_lock): Remove variable.
(_dl_static_init): Remove _dl_static_lock locking.
ports/ChangeLog.mips
* sysdeps/unix/sysv/linux/mips/dl-static.c: Do not include
<bits/libc-lock.h>.
(_dl_static_lock): Remove variable.
(_dl_static_init): Remove _dl_static_lock locking.
Maciej
glibc-static-dl-init-lock.diff
Index: glibc-fsf-trunk-quilt/ports/sysdeps/unix/sysv/linux/ia64/dl-static.c
===================================================================
--- glibc-fsf-trunk-quilt.orig/ports/sysdeps/unix/sysv/linux/ia64/dl-static.c 2013-06-18 01:05:31.000000000 +0100
+++ glibc-fsf-trunk-quilt/ports/sysdeps/unix/sysv/linux/ia64/dl-static.c 2013-06-21 19:54:31.852772816 +0100
@@ -35,9 +35,6 @@ _dl_var_init (void *array[])
}
#else
-#include <bits/libc-lock.h>
-
-__libc_lock_define_initialized_recursive (static, _dl_static_lock)
static void *variables[] =
{
@@ -52,8 +49,6 @@ _dl_static_init (struct link_map *map)
lookup_t loadbase;
void (*f) (void *[]);
- __libc_lock_lock_recursive (_dl_static_lock);
-
loadbase = _dl_lookup_symbol_x ("_dl_var_init", map, &ref,
map->l_local_scope, NULL, 0, 1, NULL);
if (ref != NULL)
@@ -61,8 +56,6 @@ _dl_static_init (struct link_map *map)
f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
f (variables);
}
-
- __libc_lock_unlock_recursive (_dl_static_lock);
}
#endif
Index: glibc-fsf-trunk-quilt/ports/sysdeps/unix/sysv/linux/mips/dl-static.c
===================================================================
--- glibc-fsf-trunk-quilt.orig/ports/sysdeps/unix/sysv/linux/mips/dl-static.c 2013-06-18 01:05:31.000000000 +0100
+++ glibc-fsf-trunk-quilt/ports/sysdeps/unix/sysv/linux/mips/dl-static.c 2013-06-21 19:54:51.852817334 +0100
@@ -33,9 +33,6 @@ _dl_var_init (void *array[])
}
#else
-#include <bits/libc-lock.h>
-
-__libc_lock_define_initialized_recursive (static, _dl_static_lock)
static void *variables[] =
{
@@ -64,8 +61,6 @@ _dl_static_init (struct link_map *l)
void (*f) (void *[]);
size_t i;
- __libc_lock_lock_recursive (_dl_static_lock);
-
loadbase = _dl_lookup_symbol_x ("_dl_var_init", l, &ref, l->l_local_scope,
NULL, 0, 1, NULL);
@@ -84,8 +79,6 @@ _dl_static_init (struct link_map *l)
f (variables);
_dl_protect_relro (rtld_map);
}
-
- __libc_lock_unlock_recursive (_dl_static_lock);
}
#endif