From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6583 invoked by alias); 15 Apr 2004 18:28:12 -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 6567 invoked from network); 15 Apr 2004 18:28:11 -0000 Received: from unknown (HELO palrel13.hp.com) (156.153.255.238) by sources.redhat.com with SMTP; 15 Apr 2004 18:28:11 -0000 Received: from hplms2.hpl.hp.com (hplms2.hpl.hp.com [15.0.152.33]) by palrel13.hp.com (Postfix) with ESMTP id 4BECA1C00580; Thu, 15 Apr 2004 11:28:11 -0700 (PDT) Received: from napali.hpl.hp.com (napali.hpl.hp.com [15.4.89.123]) by hplms2.hpl.hp.com (8.12.10/8.12.10/HPL-PA Hub) with ESMTP id i3FIS9L6020220; Thu, 15 Apr 2004 11:28:09 -0700 (PDT) Received: from napali.hpl.hp.com (napali [127.0.0.1]) by napali.hpl.hp.com (8.12.11/8.12.11/Debian-1) with ESMTP id i3FIS9mk016828; Thu, 15 Apr 2004 11:28:09 -0700 Received: (from davidm@localhost) by napali.hpl.hp.com (8.12.11/8.12.11/Debian-1) id i3FIS9Bf016824; Thu, 15 Apr 2004 11:28:09 -0700 From: David Mosberger MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="oFEec5w0Hp" Content-Transfer-Encoding: 7bit Message-ID: <16510.54329.104976.958398@napali.hpl.hp.com> Date: Thu, 15 Apr 2004 18:28:00 -0000 To: roland@redhat.com, libc-hacker@sources.redhat.com Subject: Re: second thoughts on using dl_iterate_phdr() for cache-validation Reply-To: davidm@hpl.hp.com X-URL: http://www.hpl.hp.com/personal/David_Mosberger/ X-SW-Source: 2004-04/txt/msg00031.txt.bz2 --oFEec5w0Hp Content-Type: text/plain; charset=us-ascii Content-Description: message body text Content-Transfer-Encoding: 7bit Content-length: 93 Roland, Any comments on the proposal? I'd really like to wrap this up. Thanks, --david --oFEec5w0Hp Content-Type: message/rfc822 Content-Description: forwarded message Content-Transfer-Encoding: 7bit Content-length: 10284 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-Path: Received: from hplms2.hpl.hp.com (root@hplms2.hpl.hp.com [15.0.152.33]) by napali.hpl.hp.com (8.12.11/8.12.11/Debian-1) with ESMTP id i312WwNQ006287 for ; Wed, 31 Mar 2004 18:32:58 -0800 Received: from napali.hpl.hp.com (napali.hpl.hp.com [15.4.89.123]) by hplms2.hpl.hp.com (8.12.10/8.12.10/HPL-PA Hub) with ESMTP id i312WvAC014944; Wed, 31 Mar 2004 18:32:58 -0800 (PST) Received: from napali.hpl.hp.com (napali [127.0.0.1]) by napali.hpl.hp.com (8.12.11/8.12.11/Debian-1) with ESMTP id i312WvrJ006282; Wed, 31 Mar 2004 18:32:57 -0800 Received: (from davidm@localhost) by napali.hpl.hp.com (8.12.11/8.12.11/Debian-1) id i312Wv37006278; Wed, 31 Mar 2004 18:32:57 -0800 Message-ID: <16491.32601.193260.288217@napali.hpl.hp.com> In-Reply-To: <200403310804.i2V84PP6007330@magilla.sf.frob.com> References: <16490.8972.453326.222897@napali.hpl.hp.com> <200403310804.i2V84PP6007330@magilla.sf.frob.com> X-Mailer: VM 7.18 under Emacs 21.3.1 From: David Mosberger 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 Date: Wed, 31 Mar 2004 18:32:57 -0800 Reply-To: davidm@hpl.hp.com X-URL: http://www.hpl.hp.com/personal/David_Mosberger/ Content-length: 8875 >>>>> On Wed, 31 Mar 2004 00:04:25 -0800, Roland McGrath said: Roland> The only thing I see is to give you a way to see Roland> _dl_load_adds without calling __dl_iterate_phdr and taking Roland> locks, which is what you asked for in the first place. We Roland> really can't give you an exported variable symbol, so it Roland> would have to be a new function interface. How about something along the lines of this: ----------- /* The value returned by this function increments by at least 1 when objects are added to the list visited by dl_iterate_phdr(). This function is not 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 dl_phdr_additions_counter (void); /* The vaue returned by this function increments by at least 1 when objects are removed from the list visited by dl_iterate_phdr(). This function is not 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 dl_phdr_removals_counter (void); ----------- I considered using a single function with an argument which specifies the counter to read, but then I thought that many programs may only read one or the other counter. Plus if there is two counters, someone might want to add a third counter in the future and if that needs to be supported, there would have to be a way for detecting whether a libc supports the new counter, etc., so I thought having two separate routines might not be such a bad idea after all. A preliminary but complete patch is below and it dropped the cost of unw_init_local() from 223 to 119ns (on the faster machine I profiled yesterday). The code of unw_init_local() itself accounts for 113ns and the overhead of 6ns is about in line with a shared library call. This performance-level seems very acceptable to me. Note that the way things work in libunwind, I don't have to worry about concurrent dl_close() operations and hence it is safe to read the counter without locking. Some questions/comments: - I always get confused about the versioning stuff in glibc. I added the new functions to elf/Versions, but am not sure this is really correct. - I wasn't sure what the right place for the new functions is. For now, I put them into dl-iteratephdr.c which is probably a bad choice. Furthermore, I'm not sure about name-space pollution issues. Advice on the naming etc. would be welcome. - For now, I didn't remove the dl_adds/dl_subs members from the dl_iterate_phdr() callback. With the new interface, they won't be needed anymore and I suspect nobody other than libunwind would be affected if we removed them (and even old libunwind versions would continue work properly). If you agree, I'll cook up a second patch to remove this stuff once the new interface is in. Thanks, --david Index: elf/Versions =================================================================== RCS file: /cvs/glibc/libc/elf/Versions,v retrieving revision 1.51 diff -u -r1.51 Versions --- elf/Versions 6 Mar 2004 08:04:12 -0000 1.51 +++ elf/Versions 1 Apr 2004 02:18:54 -0000 @@ -11,6 +11,10 @@ GLIBC_2.2.4 { dl_iterate_phdr; } + GLIBC_2.3.2 { + dl_phdr_additions_counter; + dl_phdr_removals_counter; + } %ifdef EXPORT_UNWIND_FIND_FDE GCC_3.0 { __register_frame_info_bases; __deregister_frame_info_bases; Index: elf/dl-close.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-close.c,v retrieving revision 1.103 diff -u -r1.103 dl-close.c --- elf/dl-close.c 7 Mar 2004 08:38:43 -0000 1.103 +++ elf/dl-close.c 1 Apr 2004 02:18:55 -0000 @@ -319,6 +319,7 @@ /* Notify the debugger we are about to remove some loaded objects. */ _r_debug.r_state = RT_DELETE; GLRO(dl_debug_state) (); + ++GL(dl_load_subs); #ifdef USE_TLS size_t tls_free_start; Index: elf/dl-iteratephdr.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-iteratephdr.c,v retrieving revision 1.11 diff -u -r1.11 dl-iteratephdr.c --- elf/dl-iteratephdr.c 27 Jan 2004 21:29:22 -0000 1.11 +++ elf/dl-iteratephdr.c 1 Apr 2004 02:18:55 -0000 @@ -23,6 +23,19 @@ #include #include + +unsigned long long +dl_phdr_additions_counter (void) +{ + return GL(dl_load_adds); +} + +unsigned long long +dl_phdr_removals_counter (void) +{ + return GL(dl_load_subs); +} + static void cancel_handler (void *arg __attribute__((unused))) { Index: elf/dl-support.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-support.c,v retrieving revision 1.83 diff -u -r1.83 dl-support.c --- elf/dl-support.c 6 Mar 2004 09:06:15 -0000 1.83 +++ elf/dl-support.c 1 Apr 2004 02:18:55 -0000 @@ -75,6 +75,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/tst-dlmodcount.c =================================================================== RCS file: /cvs/glibc/libc/elf/tst-dlmodcount.c,v retrieving revision 1.3 diff -u -r1.3 tst-dlmodcount.c --- elf/tst-dlmodcount.c 26 Mar 2004 09:48:53 -0000 1.3 +++ elf/tst-dlmodcount.c 1 Apr 2004 02:18:55 -0000 @@ -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 =================================================================== RCS file: /cvs/glibc/libc/include/link.h,v retrieving revision 1.31 diff -u -r1.31 link.h --- include/link.h 27 Mar 2004 03:22:29 -0000 1.31 +++ include/link.h 1 Apr 2004 02:18:55 -0000 @@ -307,4 +307,18 @@ size_t size, void *data), void *data); +/* The value returned by this function increments by at least 1 when + objects are added to the list visited by dl_iterate_phdr(). This + function is not 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 dl_phdr_additions_counter (void); + +/* The vaue returned by this function increments by at least 1 when + objects are removed from the list visited by dl_iterate_phdr(). + This function is not 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 dl_phdr_removals_counter (void); + #endif /* link.h */ Index: sysdeps/generic/ldsodefs.h =================================================================== RCS file: /cvs/glibc/libc/sysdeps/generic/ldsodefs.h,v retrieving revision 1.100 diff -u -r1.100 ldsodefs.h --- sysdeps/generic/ldsodefs.h 27 Mar 2004 03:23:51 -0000 1.100 +++ sysdeps/generic/ldsodefs.h 1 Apr 2004 02:18:59 -0000 @@ -241,6 +241,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; --oFEec5w0Hp--