]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
pthread: Refactor semaphore code
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>
Wed, 3 Feb 2021 16:50:21 +0000 (13:50 -0300)
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>
Mon, 8 Feb 2021 17:10:42 +0000 (14:10 -0300)
The internal semaphore list code is moved to a specific file,
sem_routine.c, and the internal usage is simplified to only two
functions (one to insert a new semaphore and one to remove it
from the internal list).  There is no need to expose the
internal locking, neither how the semaphore mapping is implemented.

No functional or semantic change is expected, tested on
x86_64-linux-gnu.

htl/Makefile
htl/semaphoreP.h
nptl/Makefile
nptl/semaphoreP.h
sysdeps/pthread/sem_close.c
sysdeps/pthread/sem_open.c
sysdeps/pthread/sem_routines.c [new file with mode: 0644]
sysdeps/pthread/sem_routines.h [new file with mode: 0644]

index 9adc95e07f5ac42e5c3af5b13593122e99ad18ee..c15c1b194eaf93dd6a9ac6fb9319454a318fe41b 100644 (file)
@@ -131,6 +131,7 @@ libpthread-routines := pt-attr pt-attr-destroy pt-attr-getdetachstate           \
        sem_close sem-destroy sem-getvalue sem-init sem_open                \
        sem-post sem-timedwait sem-trywait sem_unlink                       \
        sem-wait sem-waitfast                                               \
+       sem_routines                                                        \
                                                                            \
        cancellation                                                        \
        cthreads-compat                                                     \
index 3d6efcdec7a5ab33eb7079470d5297e3844dc695..4b13f7b30d2f7930d88325e934cff182299636a1 100644 (file)
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
   /* This always is a shared semaphore.  */
index c49134b93697ab0f13fb20c3df892f902c1a1884..8fb7fee6dbbad79f809318c028a7544ae5db48cb 100644 (file)
@@ -137,7 +137,7 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
                      pthread_once \
                      old_pthread_atfork \
                      pthread_getcpuclockid \
-                     sem_init sem_destroy \
+                     sem_init sem_destroy sem_routines \
                      sem_open sem_close sem_unlink \
                      sem_getvalue \
                      sem_wait sem_timedwait sem_clockwait sem_post \
index 3585af09fcf5a294a4476393701150bc4a8057df..1b786149f48c9acdf569b93d91bb5fe2182cdf79 100644 (file)
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
 #if __HAVE_64B_ATOMICS
index 8b6c9c6dc5f86a673772a5755e4c6d2ce96d337f..6649196cac54b33ee74285761a04a525f8f732e9 100644 (file)
    <https://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
-#include <search.h>
-#include <sys/mman.h>
 #include "semaphoreP.h"
-
-struct walk_closure
-{
-  sem_t *the_sem;
-  struct inuse_sem *rec;
-};
-
-static void
-walker (const void *inodep, VISIT which, void *closure0)
-{
-  struct walk_closure *closure = closure0;
-  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
-
-  if (nodep->sem == closure->the_sem)
-    closure->rec = nodep;
-}
-
+#include <sem_routines.h>
 
 int
 sem_close (sem_t *sem)
 {
-  int result = 0;
-
-  /* Get the lock.  */
-  lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-  /* Locate the entry for the mapping the caller provided.  */
-  struct inuse_sem *rec;
-  {
-    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
-    __twalk_r (__sem_mappings, walker, &closure);
-    rec = closure.rec;
-  }
-  if  (rec != NULL)
+  if (!__sem_remove_mapping (sem))
     {
-      /* Check the reference counter.  If it is going to be zero, free
-        all the resources.  */
-      if (--rec->refcnt == 0)
-       {
-         /* Remove the record from the tree.  */
-         (void) __tdelete (rec, &__sem_mappings, __sem_search);
-
-         result = munmap (rec->sem, sizeof (sem_t));
-
-         free (rec);
-       }
-    }
-  else
-    {
-      /* This is no valid semaphore.  */
-      result = -1;
       __set_errno (EINVAL);
+      return -1;
     }
 
-  /* Release the lock.  */
-  lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-
-  return result;
+  return 0;
 }
index d666414f3262e1117104d73ec57ca06603052821..028d76a685b2f3e34c71684561ac17fbab0b0993 100644 (file)
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
 #include <fcntl.h>
-#include <pthread.h>
-#include <search.h>
 #include <semaphore.h>
 #include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <unistd.h>
 #include <sys/mman.h>
-#include <sys/stat.h>
 #include "semaphoreP.h"
 #include <shm-directory.h>
+#include <sem_routines.h>
 #include <futex-internal.h>
 #include <libc-lock.h>
 
-/* Comparison function for search of existing mapping.  */
-int
-attribute_hidden
-__sem_search (const void *a, const void *b)
-{
-  const struct inuse_sem *as = (const struct inuse_sem *) a;
-  const struct inuse_sem *bs = (const struct inuse_sem *) b;
-
-  if (as->ino != bs->ino)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
-
-  if (as->dev != bs->dev)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
-
-  return strcmp (as->name, bs->name);
-}
-
-
-/* The search tree for existing mappings.  */
-void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-int __sem_mappings_lock attribute_hidden = LLL_LOCK_INITIALIZER;
-
-
-/* Search for existing mapping and if possible add the one provided.  */
-static sem_t *
-check_add_mapping (const char *name, int fd, sem_t *existing)
-{
-  size_t namelen = strlen (name);
-  sem_t *result = SEM_FAILED;
-
-  /* Get the information about the file.  */
-  struct stat64 st;
-  if (__fstat64 (fd, &st) == 0)
-    {
-      /* Get the lock.  */
-      lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-      /* Search for an existing mapping given the information we have.  */
-      struct inuse_sem *fake;
-      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
-      memcpy (fake->name, name, namelen);
-      fake->dev = st.st_dev;
-      fake->ino = st.st_ino;
-
-      struct inuse_sem **foundp = __tfind (fake, &__sem_mappings,
-                                          __sem_search);
-      if (foundp != NULL)
-       {
-         /* There is already a mapping.  Use it.  */
-         result = (*foundp)->sem;
-         ++(*foundp)->refcnt;
-       }
-      else
-       {
-         /* We haven't found a mapping.  Install ione.  */
-         struct inuse_sem *newp;
-
-         newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
-         if (newp != NULL)
-           {
-             /* If the caller hasn't provided any map it now.  */
-             if (existing == SEM_FAILED)
-               existing = (sem_t *) mmap (NULL, sizeof (sem_t),
-                                          PROT_READ | PROT_WRITE, MAP_SHARED,
-                                          fd, 0);
-
-             newp->dev = st.st_dev;
-             newp->ino = st.st_ino;
-             newp->refcnt = 1;
-             newp->sem = existing;
-             memcpy (newp->name, name, namelen);
-
-             /* Insert the new value.  */
-             if (existing != MAP_FAILED
-                 && __tsearch (newp, &__sem_mappings, __sem_search) != NULL)
-               /* Successful.  */
-               result = existing;
-             else
-               /* Something went wrong while inserting the new
-                  value.  We fail completely.  */
-               free (newp);
-           }
-       }
-
-      /* Release the lock.  */
-      lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-    }
-
-  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
-    {
-      /* Do not disturb errno.  */
-      int save = errno;
-      munmap (existing, sizeof (sem_t));
-      errno = save;
-    }
-
-  return result;
-}
-
-
 sem_t *
 sem_open (const char *name, int oflag, ...)
 {
@@ -183,7 +73,7 @@ sem_open (const char *name, int oflag, ...)
       else
        /* Check whether we already have this semaphore mapped and
           create one if necessary.  */
-       result = check_add_mapping (name, fd, SEM_FAILED);
+       result = __sem_check_add_mapping (name, fd, SEM_FAILED);
     }
   else
     {
@@ -294,7 +184,7 @@ sem_open (const char *name, int oflag, ...)
            /* Insert the mapping into the search tree.  This also
               determines whether another thread sneaked by and already
               added such a mapping despite the fact that we created it.  */
-           result = check_add_mapping (name, fd, result);
+           result = __sem_check_add_mapping (name, fd, result);
        }
 
       /* Now remove the temporary name.  This should never fail.  If
diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
new file mode 100644 (file)
index 0000000..78d9364
--- /dev/null
@@ -0,0 +1,187 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <search.h>
+#include <semaphoreP.h>
+#include <sys/mman.h>
+#include <sem_routines.h>
+
+/* Keeping track of currently used mappings.  */
+struct inuse_sem
+{
+  dev_t dev;
+  ino_t ino;
+  int refcnt;
+  sem_t *sem;
+  char name[];
+};
+
+/* Comparison function for search of existing mapping.  */
+static int
+sem_search (const void *a, const void *b)
+{
+  const struct inuse_sem *as = (const struct inuse_sem *) a;
+  const struct inuse_sem *bs = (const struct inuse_sem *) b;
+
+  if (as->ino != bs->ino)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
+
+  if (as->dev != bs->dev)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
+
+  return strcmp (as->name, bs->name);
+}
+
+/* The search tree for existing mappings.  */
+static void *sem_mappings;
+
+/* Lock to protect the search tree.  */
+static int sem_mappings_lock = LLL_LOCK_INITIALIZER;
+
+
+/* Search for existing mapping and if possible add the one provided.  */
+sem_t *
+__sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+{
+  size_t namelen = strlen (name);
+  sem_t *result = SEM_FAILED;
+
+  /* Get the information about the file.  */
+  struct stat64 st;
+  if (__fstat64 (fd, &st) == 0)
+    {
+      /* Get the lock.  */
+      lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+      /* Search for an existing mapping given the information we have.  */
+      struct inuse_sem *fake;
+      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
+      memcpy (fake->name, name, namelen);
+      fake->dev = st.st_dev;
+      fake->ino = st.st_ino;
+
+      struct inuse_sem **foundp = __tfind (fake, &sem_mappings, sem_search);
+      if (foundp != NULL)
+       {
+         /* There is already a mapping.  Use it.  */
+         result = (*foundp)->sem;
+         ++(*foundp)->refcnt;
+       }
+      else
+       {
+         /* We haven't found a mapping.  Install ione.  */
+         struct inuse_sem *newp;
+
+         newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
+         if (newp != NULL)
+           {
+             /* If the caller hasn't provided any map it now.  */
+             if (existing == SEM_FAILED)
+               existing = (sem_t *) mmap (NULL, sizeof (sem_t),
+                                          PROT_READ | PROT_WRITE, MAP_SHARED,
+                                          fd, 0);
+
+             newp->dev = st.st_dev;
+             newp->ino = st.st_ino;
+             newp->refcnt = 1;
+             newp->sem = existing;
+             memcpy (newp->name, name, namelen);
+
+             /* Insert the new value.  */
+             if (existing != MAP_FAILED
+                 && __tsearch (newp, &sem_mappings, sem_search) != NULL)
+               /* Successful.  */
+               result = existing;
+             else
+               /* Something went wrong while inserting the new
+                  value.  We fail completely.  */
+               free (newp);
+           }
+       }
+
+      /* Release the lock.  */
+      lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+    }
+
+  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
+    {
+      /* Do not disturb errno.  */
+      int save = errno;
+      munmap (existing, sizeof (sem_t));
+      errno = save;
+    }
+
+  return result;
+}
+
+struct walk_closure
+{
+  sem_t *the_sem;
+  struct inuse_sem *rec;
+};
+
+static void
+walker (const void *inodep, VISIT which, void *closure0)
+{
+  struct walk_closure *closure = closure0;
+  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
+
+  if (nodep->sem == closure->the_sem)
+    closure->rec = nodep;
+}
+
+bool
+__sem_remove_mapping (sem_t *sem)
+{
+  bool ret = true;
+
+  /* Get the lock.  */
+  lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+  /* Locate the entry for the mapping the caller provided.  */
+  struct inuse_sem *rec;
+  {
+    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
+    __twalk_r (sem_mappings, walker, &closure);
+    rec = closure.rec;
+  }
+  if (rec != NULL)
+    {
+      /* Check the reference counter.  If it is going to be zero, free
+        all the resources.  */
+      if (--rec->refcnt == 0)
+       {
+         /* Remove the record from the tree.  */
+         __tdelete (rec, &sem_mappings, sem_search);
+
+         if (munmap (rec->sem, sizeof (sem_t)) == -1)
+           ret = false;
+
+         free (rec);
+       }
+    }
+  else
+    ret = false;
+
+  /* Release the lock.  */
+  lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+
+  return ret;
+}
diff --git a/sysdeps/pthread/sem_routines.h b/sysdeps/pthread/sem_routines.h
new file mode 100644 (file)
index 0000000..25d3e88
--- /dev/null
@@ -0,0 +1,27 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SEM_ROUTINES_H
+#define _SEM_ROUTINES_H
+
+sem_t * __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+  attribute_hidden;
+
+bool __sem_remove_mapping (sem_t *sem) attribute_hidden;
+
+#endif