Re: second thoughts on using dl_iterate_phdr() for cache-validation

[David Mosberger <davidm@napali.hpl.hp.com>]

This is a follow-up on a discussion that went dead around March 31.
There were no objections to the patch I presented then and if that's
still the case, I'd appreciate if this patch could be checked in.  It
speeds up dynamic-object cache-validition a great deal (~ a factor of
2, mainly because dl_iterate_phdr() requires taking an expensive
lock).

BTW: i assume an interface along the lines of:

 enum dl_counter {
   DL_PHDR_ADDITIONS,
   DL_PHDR_REMOVALS
 };

 unsigned long long *dl_get_counter_address (enum dl_counter which_counter);

is out of the question.  If not, this would be even better, since the
cache-validation overhead would drop to a simple load & compare.

Thanks,

	--david

---
ChangeLog

2004-11-04  David Mosberger  <davidm@hpl.hp.com>

	* elf/Versions (dl_phdr_additions_count): New export
	(dl_phdr_removals_count): Likewise.
	* elf/dl-close.c (_dl_close): Increment dl_load_subs;
	* elf/dl-iteratephdr.c (dl_phdr_additions_count): New function.
	(dl_phdr_removals_count): Likewise.
	* elf/dl-support.c: New variable.
	* elf/tst-dlmodcount.c: New file.
	* include/link.h (dl_phdr_additions_count): Declare.
	(dl_phdr_removals_count): Likewise.
	* sysdeps/generic/ldsodefs.h (struct rtld_global): Add member
	  "_dl_load_subs".

Index: elf/Versions
===================================================================
RCS file: /cvs/glibc/libc/elf/Versions,v
retrieving revision 1.52
diff -u -r1.52 Versions
--- elf/Versions	19 Oct 2004 22:20:33 -0000	1.52
+++ elf/Versions	4 Nov 2004 23:38:29 -0000
@@ -11,6 +11,10 @@
   GLIBC_2.2.4 {
     dl_iterate_phdr;
   }
+  GLIBC_2.3.2 {
+    dl_phdr_additions_count;
+    dl_phdr_removals_count;
+  }
 %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.107
diff -u -r1.107 dl-close.c
--- elf/dl-close.c	14 Oct 2004 09:20:57 -0000	1.107
+++ elf/dl-close.c	4 Nov 2004 23:38:29 -0000
@@ -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) ();
+  ++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.12
diff -u -r1.12 dl-iteratephdr.c
--- elf/dl-iteratephdr.c	14 Oct 2004 02:02:06 -0000	1.12
+++ elf/dl-iteratephdr.c	4 Nov 2004 23:38:29 -0000
@@ -23,6 +23,19 @@
 #include <stddef.h>
 #include <bits/libc-lock.h>
 
+
+unsigned long long
+dl_phdr_additions_count (void)
+{
+  return GL(dl_load_adds);
+}
+
+unsigned long long
+dl_phdr_removals_count (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.86
diff -u -r1.86 dl-support.c
--- elf/dl-support.c	14 Oct 2004 02:06:18 -0000	1.86
+++ elf/dl-support.c	4 Nov 2004 23:38:29 -0000
@@ -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/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	4 Nov 2004 23:38:29 -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_count (), dl_phdr_removals_count ());
+
+  switch (cmd)
+    {
+    case SET:
+      break;
+
+    case ADD:
+      if (leq (dl_phdr_additions_count (), last_adds))
+	{
+	  fprintf (stderr, "dlpi_adds failed to get incremented!\n");
+	  exit (3);
+	}
+      break;
+
+    case REMOVE:
+      if (leq (dl_phdr_removals_count (), last_subs))
+	{
+	  fprintf (stderr, "dlpi_subs failed to get incremented!\n");
+	  exit (4);
+	}
+      break;
+    }
+  last_adds = dl_phdr_additions_count ();
+  last_subs = dl_phdr_removals_count ();
+}
+
 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.33
diff -u -r1.33 link.h
--- include/link.h	14 Oct 2004 01:57:54 -0000	1.33
+++ include/link.h	4 Nov 2004 23:38:30 -0000
@@ -317,4 +317,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_count (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_count (void);
+
 #endif /* link.h */
Index: sysdeps/generic/ldsodefs.h
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/ldsodefs.h,v
retrieving revision 1.105
diff -u -r1.105 ldsodefs.h
--- sysdeps/generic/ldsodefs.h	19 Oct 2004 22:19:17 -0000	1.105
+++ sysdeps/generic/ldsodefs.h	4 Nov 2004 23:38:31 -0000
@@ -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;