This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix nptl semaphore cleanup invocation
From: Roland McGrath <roland@hack.frob.com>
Date: Fri, 2 Sep 2011 11:47:18 -0700 (PDT)
> Also some failures of whitespace discipline.
> But as to the substance, I can't really say yay or nay.
Here is an updated version with code comments added.
Could you be more specific about the whitespace issues? I cut and
pasted the code, reformatted with emacs in GNU coding style mode, and
the function declaration matches exactly how this style of function
with attributes is declared elsewhere in the glibc tree.
nptl/
2011-09-02 David S. Miller <davem@davemloft.net>
* sysdeps/unix/sysv/linux/sem_timedwait.c (do_futex_timed_wait):
New function.
(sem_timedwait): Call it to force an exception region around
the async cancel enable and the futex operation.
* sysdeps/unix/sysv/linux/sparc/sem_timedwait.c: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c: Likewise.
* sysdeps/unix/sysv/linux/sem_wait.c (do_futex_wait): New function.
(__new_sem_wait): Call it to force an exception region around
the async cancel enable and the futex operation.
* sysdeps/unix/sysv/linux/sparc/sem_wait.c: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c: Likewise.
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
index fdf0d74..3b50107 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
@@ -30,6 +30,21 @@
extern void __sem_wait_cleanup (void *arg) attribute_hidden;
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct new_sem *isem, struct timespec *rt)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_timed_wait (&isem->value, 0, rt,
+ isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -80,16 +95,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
/* Do wait. */
rt.tv_sec = sec;
rt.tv_nsec = nsec;
-
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_timed_wait (&isem->value, 0, &rt,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_timed_wait(isem, &rt);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
index 20e2b48..4fc0d1d 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
@@ -37,6 +37,20 @@ __sem_wait_cleanup (void *arg)
atomic_decrement (&isem->nwaiters);
}
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct new_sem *isem)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
__new_sem_wait (sem_t *sem)
@@ -53,15 +67,7 @@ __new_sem_wait (sem_t *sem)
while (1)
{
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_wait (&isem->value, 0,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_wait(isem);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
index 01952f3..22554df 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
@@ -30,6 +30,21 @@
extern void __sem_wait_cleanup (void *arg) attribute_hidden;
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct sparc_new_sem *isem, struct timespec *rt)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_timed_wait (&isem->value, 0, rt,
+ isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -80,16 +95,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
/* Do wait. */
rt.tv_sec = sec;
rt.tv_nsec = nsec;
-
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_timed_wait (&isem->value, 0, &rt,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_timed_wait(isem, &rt);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
index a846f20..f950187 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
@@ -37,6 +37,20 @@ __sem_wait_cleanup (void *arg)
atomic_decrement (&isem->nwaiters);
}
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct sparc_new_sem *isem)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
__new_sem_wait (sem_t *sem)
@@ -53,15 +67,7 @@ __new_sem_wait (sem_t *sem)
while (1)
{
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_wait (&isem->value, 0,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_wait(isem);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
index 55f3e2e..9a8d958 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
@@ -30,6 +30,21 @@
extern void __sem_wait_cleanup (void *arg) attribute_hidden;
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct sparc_new_sem *isem, struct timespec *rt)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_timed_wait (&isem->value, 0, rt,
+ isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -99,16 +114,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
/* Do wait. */
rt.tv_sec = sec;
rt.tv_nsec = nsec;
-
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_timed_wait (&isem->value, 0, &rt,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_timed_wait(isem, &rt);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
index b14f976..3fd11ec 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
@@ -44,6 +44,20 @@ __sem_wait_cleanup (void *arg)
}
}
+/* This is in a seperate function in order to make sure gcc
+ puts the call site into an exception region, and thus the
+ cleanups get properly run. */
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct sparc_new_sem *isem)
+{
+ int err, oldtype = __pthread_enable_asynccancel ();
+
+ err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+ __pthread_disable_asynccancel (oldtype);
+ return err;
+}
int
__new_sem_wait (sem_t *sem)
@@ -77,15 +91,7 @@ __new_sem_wait (sem_t *sem)
while (1)
{
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
- err = lll_futex_wait (&isem->value, 0,
- isem->private ^ FUTEX_PRIVATE_FLAG);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
-
+ err = do_futex_wait(isem);
if (err != 0 && err != -EWOULDBLOCK)
{
__set_errno (-err);