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

[David Mosberger <davidm at napali dot hpl dot hp dot com>]

>>>>> On Mon, 8 Nov 2004 20:40:15 -0800, Roland McGrath <roland@redhat.com> said:

  >> >>>>> On Fri, 5 Nov 2004 18:32:18 -0800, Roland McGrath
  >> <roland@redhat.com> said:

  Roland> For platforms where it doesn't work, you can use nonblocking
  Roland> synchronization a la seqcount.

Below is a version that uses Keith Owens' (et al) seqlock approach to
support the counters on platforms where "unsigned long long" is bigger
than "unsigned long".  Since the writers are already serialized via a
lock, this should be very efficient and still correct.

Please apply, if there are no further objections.

Thanks,

	--david

 ChangeLog                  |   19 +++++
 Versions.def               |    1 
 elf/Makefile               |    2 
 elf/Versions               |    4 +
 elf/dl-close.c             |    1 
 elf/dl-counters.c          |  121 +++++++++++++++++++++++++++++++++++
 elf/dl-object.c            |    2 
 elf/dl-support.c           |    3 
 elf/rtld.c                 |    2 
 elf/tst-dlmodcount.c       |   36 ++++++++++
 include/link.h             |   19 +++++
 sysdeps/generic/ldsodefs.h |    6 +
 16 files changed, 280 insertions(+), 130 deletions(-)

ChangeLog

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

	* 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.
	* 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;
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 <davidm@hpl.hp.com>, 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 <errno.h>
+#include <ldsodefs.h>
+#include <stddef.h>
+#include <atomic.h>
+
+/* 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 */