diff mbox series

[3/8] elf: Avoid redundant ld.so.cache reload after first load

Message ID 20260706211524.3801466-4-adhemerval.zanella@linaro.org (mailing list archive)
State Committed
Commit f1c94392d96ba10b6ea3acbd617f73d5fab35b88
Headers
Series Fixes for the system-wide tunables support |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Netto July 6, 2026, 9:14 p.m. UTC 
  _dl_check_ldsocache_needs_loading only stored the stat fields it
compares (mtime, ino, size, dev) on the path where a cache was already
loaded.  On the very first call CACHE is NULL and the function returned
"needs loading" without recording those fields, leaving
new_cache_file_time zero.  The next call then copied that zero value
into cache_file_time and compared it against the freshly stat'd values,
which always differed, forcing a second, unnecessary load (munmap +
mmap + re-parse) of an unchanged cache at every startup.

It can be shown with repro:

  $ cat << EOF > repro.c
  #include <dlfcn.h>
  int main (void) { dlopen ("does-not-exist-xyz.so.99", RTLD_NOW); return 0; }
  EOF
  $ gcc repro.c -o repro
  $ strace -f -e trace=openat elf/ld.so --library-path . ./repro 2>&1 | grep -c "/etc/ld.so.cache"

The result should be 1, instead of 2.

Record the stat fields as soon as the stat succeeds, before the
CACHE == NULL early return, so the following call has an accurate
baseline and does not spuriously reload.
---
 elf/dl-cache.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

DJ Delorie July 9, 2026, 2:06 a.m. UTC | #1 
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> -  /* Any file is better than no file (likely the first time
> -     through).  */
> -  if (cache == NULL)
> -    return true;
> -
> -  /* Store the fields we check, in order they're likely to differ.  */
> +  /* Store the fields we check, in order they're likely to differ.  We
> +     must do this even for the first load (CACHE == NULL below), so that
> +     the next call copies an accurate NEW_CACHE_FILE_TIME into
> +     CACHE_FILE_TIME and does not spuriously reload the unchanged cache.  */
>    new_cache_file_time.mtime = new_cache_file_stat.st_mtime;
>    new_cache_file_time.ino = new_cache_file_stat.st_ino;
>    new_cache_file_time.size = new_cache_file_stat.st_size;
>    new_cache_file_time.dev = new_cache_file_stat.st_dev;
>  
> +  /* Any file is better than no file (likely the first time
> +     through).  */
> +  if (cache == NULL)
> +    return true;
> +
>    /* At this point, NEW_CACHE_FILE_TIME is valid as well as
>       CACHE_FILE_TIME, so we compare them.  */
>    return (memcmp (&new_cache_file_time, &cache_file_time,

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
diff mbox series

Patch

diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 498196d0aa3..03bcd5bd06e 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -433,17 +433,20 @@  _dl_check_ldsocache_needs_loading (void)
   if (rv < 0)
     return false;
 
-  /* Any file is better than no file (likely the first time
-     through).  */
-  if (cache == NULL)
-    return true;
-
-  /* Store the fields we check, in order they're likely to differ.  */
+  /* Store the fields we check, in order they're likely to differ.  We
+     must do this even for the first load (CACHE == NULL below), so that
+     the next call copies an accurate NEW_CACHE_FILE_TIME into
+     CACHE_FILE_TIME and does not spuriously reload the unchanged cache.  */
   new_cache_file_time.mtime = new_cache_file_stat.st_mtime;
   new_cache_file_time.ino = new_cache_file_stat.st_ino;
   new_cache_file_time.size = new_cache_file_stat.st_size;
   new_cache_file_time.dev = new_cache_file_stat.st_dev;
 
+  /* Any file is better than no file (likely the first time
+     through).  */
+  if (cache == NULL)
+    return true;
+
   /* At this point, NEW_CACHE_FILE_TIME is valid as well as
      CACHE_FILE_TIME, so we compare them.  */
   return (memcmp (&new_cache_file_time, &cache_file_time,