From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8623 invoked by alias); 11 Nov 2004 06:17:17 -0000 Mailing-List: contact libc-hacker-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sources.redhat.com Received: (qmail 8603 invoked from network); 11 Nov 2004 06:17:15 -0000 Received: from unknown (HELO palrel11.hp.com) (156.153.255.246) by sourceware.org with SMTP; 11 Nov 2004 06:17:15 -0000 Received: from hplms2.hpl.hp.com (hplms2.hpl.hp.com [15.0.152.33]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by palrel11.hp.com (Postfix) with ESMTP id 50DF01CE59; Wed, 10 Nov 2004 22:17:15 -0800 (PST) Received: from napali.hpl.hp.com (napali.hpl.hp.com [15.4.89.123]) by hplms2.hpl.hp.com (8.13.1/8.13.1/HPL-PA Hub) with ESMTP id iAB6HDwQ016327; Wed, 10 Nov 2004 22:17:13 -0800 (PST) Received: from napali.hpl.hp.com (napali [127.0.0.1]) by napali.hpl.hp.com (8.13.1/8.13.1/Debian-16) with ESMTP id iAB6HCfe003926; Wed, 10 Nov 2004 22:17:13 -0800 Received: (from davidm@localhost) by napali.hpl.hp.com (8.13.1/8.13.1/Submit) id iAB6HC9g003923; Wed, 10 Nov 2004 22:17:12 -0800 From: David Mosberger MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <16787.999.696416.486337@napali.hpl.hp.com> Date: Thu, 11 Nov 2004 06:17:00 -0000 To: Roland McGrath Cc: davidm@hpl.hp.com, libc-hacker@sources.redhat.com Subject: Re: second thoughts on using dl_iterate_phdr() for cache-validation In-Reply-To: <200411102045.iAAKjoHL004770@magilla.sf.frob.com> References: <16785.43455.430509.592326@napali.hpl.hp.com> <200411102045.iAAKjoHL004770@magilla.sf.frob.com> Reply-To: davidm@hpl.hp.com X-URL: http://www.hpl.hp.com/personal/David_Mosberger/ X-SW-Source: 2004-11/txt/msg00028.txt.bz2 >>>>> On Wed, 10 Nov 2004 12:45:50 -0800, Roland McGrath said: Roland> That looks pretty reasonable. OK, thanks. I attached an updated patch which fixes the symbol-problem I was referring to in the earlier mail. I tested this patch on a freshly checked out libc tree, just to make sure it's applying properly and builds and works as expected. There were no (new) failures with the patch applied, so this one should be good to go. Roland> But we will in fact probably hold off for a while longer on Roland> adding any new interfaces rather than branching sooner. We Roland> are still getting a steady stream of good stabilizing fixes Roland> on the trunk. That's a bit disappointing. Earlier you said: Roland> If there is a complete, desireable, and working new Roland> interface ready to go in, there is no "opening date" for it. Is the attached patch falling short of being complete, desireable, and working? Thanks, --david --------------------------------------------------------------------- ChangeLog 2004-11-09 David Mosberger * Versions.def (ld): Mention GLIBC_2.3.5. * elf/Makefile (dl-routines): Mention dl-counters. * elf/Versions (dl_phdr_additions_counter): New export. (dl_phdr_removals_counter): Likewise. (_dl_increment_counter): Export as a GLIBC_PRIVATE symbol (needed by dl-close.c) * elf/dl-counters.c: New file. * elf/tst-dlmodcount.c: Add tests for dl_phdr_additions_counter() and dl_phdr_removals_counter(). * elf/dl-support.c (_dl_load_subs): New variable. * elf/dl-close.c (_dl_close): Increment dl_load_subs via a call to _dl_increment_counter(). * elf/rtld.c (dl_main): Likewise. * include/link.h (dl_phdr_additions_counter): Declare. (dl_phdr_removals_counter): Likewise. * sysdeps/generic/ldsodefs.h (struct rtld_global): Add member "_dl_load_subs". (_dl_increment_counter): Declare as hidden & internal function. Index: Versions.def --- Versions.def +++ Versions.def @@ -98,6 +98,7 @@ GLIBC_2.0 GLIBC_2.1 GLIBC_2.3 + GLIBC_2.3.5 GLIBC_PRIVATE } libthread_db { Index: elf/Makefile --- elf/Makefile +++ elf/Makefile @@ -27,7 +27,7 @@ # The core dynamic linking functions are in libc for the static and # profiled libraries. -dl-routines = $(addprefix dl-,load cache lookup object reloc deps \ +dl-routines = $(addprefix dl-,load cache counters lookup object reloc deps \ runtime error init fini debug misc \ version profile conflict tls origin \ execstack caller) Index: elf/Versions --- elf/Versions +++ elf/Versions @@ -43,6 +43,10 @@ # runtime interface to TLS __tls_get_addr; } + GLIBC_2.3.5 { + dl_phdr_additions_counter; + dl_phdr_removals_counter; + } GLIBC_PRIVATE { # Those are in the dynamic linker, but used by libc.so. __libc_enable_secure; @@ -54,6 +58,7 @@ _dl_get_tls_static_info; _dl_allocate_tls_init; _dl_tls_setup; _dl_rtld_di_serinfo; _dl_make_stack_executable; + _dl_increment_counter; # Only here for gdb while a better method is developed. _dl_debug_state; } Index: elf/dl-close.c --- elf/dl-close.c +++ elf/dl-close.c @@ -368,6 +368,7 @@ /* Notify the debugger we are about to remove some loaded objects. */ _r_debug.r_state = RT_DELETE; GLRO(dl_debug_state) (); + _dl_increment_counter (&GL(dl_load_subs)); #ifdef USE_TLS size_t tls_free_start; Index: elf/dl-counters.c --- /dev/null +++ elf/dl-counters.c @@ -0,0 +1,121 @@ +/* Access to dl-object addition/removal counters. + Copyright (C) 2004 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by David Mosberger , 2004. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If not, + write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, + Boston, MA 02111-1307, USA. */ + +#include +#include +#include +#include + +/* This seqlock is analogous to the one by Keith Owens found in the + Linux kernel, except that we don't need a spinlock since we know + that calls to _dl_increment_counter() are already serialized via + dl_load_lock. */ + +struct seqlock + { + unsigned long int sequence; + }; + + +/* Return TRUE if loads and stores to "unsigned long long int" + variables are atomic. According to Roland McGrath, Libc assumes + this to be always true for variables of type "unsigned long"... */ + +static inline int +long_long_is_atomic (void) +{ + return sizeof (unsigned long long int) == sizeof (unsigned long int); +} + +static inline void +write_seqlock (struct seqlock *sl) +{ + ++sl->sequence; + atomic_write_barrier (); +} + +static inline void +write_sequnlock (struct seqlock *sl) +{ + atomic_write_barrier (); + ++sl->sequence; +} + +static inline unsigned long int +read_seqbegin (struct seqlock *sl) +{ + unsigned long int seq = sl->sequence; + atomic_read_barrier (); + return seq; +} + +static inline int +read_seqretry (struct seqlock *sl, unsigned long int seq) +{ + atomic_read_barrier (); + return (seq & 1) | (sl->sequence ^ seq); +} + +static struct seqlock sl; + +static inline unsigned long long int +read_counter (unsigned long long int *cp) +{ + unsigned long long int val; + unsigned long int seq; + + if (long_long_is_atomic ()) + val = *cp; + else + do + { + seq = read_seqbegin (&sl); + val = *cp; + } + while (read_seqretry (&sl, seq)); + return val; +} + +void +internal_function +_dl_increment_counter (unsigned long long int *cp) +{ + if (!long_long_is_atomic ()) + write_seqlock (&sl); + + ++*cp; + + if (!long_long_is_atomic ()) + write_sequnlock (&sl); +} + +unsigned long long int +__dl_phdr_additions_counter (void) +{ + return read_counter (&GL(dl_load_adds)); +} +weak_alias (__dl_phdr_additions_counter, dl_phdr_additions_counter); + +unsigned long long int +__dl_phdr_removals_counter (void) +{ + return read_counter (&GL(dl_load_subs)); +} +weak_alias (__dl_phdr_removals_counter, dl_phdr_removals_counter); Index: elf/dl-object.c --- elf/dl-object.c +++ elf/dl-object.c @@ -85,7 +85,7 @@ else GL(dl_ns)[nsid]._ns_loaded = new; ++GL(dl_ns)[nsid]._ns_nloaded; - ++GL(dl_load_adds); + _dl_increment_counter (&GL(dl_load_adds)); /* If we have no loader the new object acts as it. */ if (loader == NULL) Index: elf/dl-support.c --- elf/dl-support.c +++ elf/dl-support.c @@ -73,6 +73,9 @@ /* Incremented whenever something may have been added to dl_loaded. */ unsigned long long _dl_load_adds; +/* Incremented whenever something may have been removed from dl_loaded. */ +unsigned long long _dl_load_subs; + /* Fake scope. In dynamically linked binaries this is the scope of the main application but here we don't have something like this. So create a fake scope containing nothing. */ Index: elf/rtld.c --- elf/rtld.c +++ elf/rtld.c @@ -1085,7 +1085,7 @@ main_map->l_next = &GL(dl_rtld_map); GL(dl_rtld_map).l_prev = main_map; ++GL(dl_ns)[LM_ID_BASE]._ns_nloaded; - ++GL(dl_load_adds); + _dl_increment_counter (&GL(dl_load_adds)); /* If LD_USE_LOAD_BIAS env variable has not been seen, default to not using bias for non-prelinked PIEs and libraries Index: elf/tst-dlmodcount.c --- elf/tst-dlmodcount.c +++ elf/tst-dlmodcount.c @@ -71,6 +71,39 @@ return -1; } +static void +check (int cmd) +{ + static int last_adds = 0, last_subs = 0; + + printf (" additions = %Lu removals = %Lu\n", + dl_phdr_additions_counter (), dl_phdr_removals_counter ()); + + switch (cmd) + { + case SET: + break; + + case ADD: + if (leq (dl_phdr_additions_counter (), last_adds)) + { + fprintf (stderr, "dlpi_adds failed to get incremented!\n"); + exit (3); + } + break; + + case REMOVE: + if (leq (dl_phdr_removals_counter (), last_subs)) + { + fprintf (stderr, "dlpi_subs failed to get incremented!\n"); + exit (4); + } + break; + } + last_adds = dl_phdr_additions_counter (); + last_subs = dl_phdr_removals_counter (); +} + static void * load (const char *path) { @@ -81,6 +114,7 @@ if (!handle) exit (1); dl_iterate_phdr (callback, (void *)(intptr_t) ADD); + check (ADD); return handle; } @@ -91,6 +125,7 @@ if (dlclose (handle) < 0) exit (2); dl_iterate_phdr (callback, (void *)(intptr_t) REMOVE); + check (REMOVE); } int @@ -99,6 +134,7 @@ void *handle1, *handle2; dl_iterate_phdr (callback, (void *)(intptr_t) SET); + check (SET); handle1 = load ("firstobj.so"); handle2 = load ("globalmod1.so"); unload ("firstobj.so", handle1); Index: include/link.h --- include/link.h +++ include/link.h @@ -317,4 +317,23 @@ size_t size, void *data), void *data); +/* The value returned by this function increments by at least 1 + (modulo N, where N=2^64 on platforms with 64-bit "long long int") + when objects are added to the list visited by dl_iterate_phdr(). + + This function may not be serialized with respect to + dl_iterate_phdr(). If serialization is needed, this function + should be called from within a dl_itereate_phdr() callback. */ +extern unsigned long long int dl_phdr_additions_counter (void); + +/* The value returned by this function increments by at least 1 + (modulo N, where N=2^64 on platforms with 64-bit "long long int") + when objects are removed from the list visited by + dl_iterate_phdr(). + + This function may not be serialized with respect to + dl_iterate_phdr(). If serialization is needed, this function + should be called from within a dl_itereate_phdr() callback. */ +extern unsigned long long int dl_phdr_removals_counter (void); + #endif /* link.h */ Index: sysdeps/generic/ldsodefs.h --- sysdeps/generic/ldsodefs.h +++ sysdeps/generic/ldsodefs.h @@ -250,6 +250,9 @@ /* Incremented whenever something may have been added to dl_loaded. */ EXTERN unsigned long long _dl_load_adds; + /* Incremented whenever something may have been removed from dl_loaded. */ + EXTERN unsigned long long _dl_load_subs; + #ifndef MAP_ANON /* File descriptor referring to the zero-fill device. */ EXTERN int _dl_zerofd; @@ -909,6 +912,9 @@ extern int _dl_check_caller (const void *caller, enum allowmask mask) attribute_hidden; +extern void _dl_increment_counter (unsigned long long int *cp); + internal_function attribute_hidden; + __END_DECLS #endif /* ldsodefs.h */