| Message ID | 1509745249-11404-6-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/6] Consolidate Linux sigprocmask implementation (BZ #22391) | expand |
On Fri, Nov 3, 2017 at 5:40 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > This patch simplify sigpause by remobing the single thread optimization > since it will be handled already by the __sigsuspend call. > > Checked on x86_64-linux-gnu. > > * sysdeps/posix/sigpause.c (do_sigpause): Remove. > (__sigpause): Rely on __sigsuspend to implement single thread > optimization. LGTM except that the addition of LIBC_CANCEL_HANDLED sent me down a rabbit hole - I might have to write a replacement for tst-cancel-wrappers.sh that doesn't depend on these magic markers - anyway, please mention in the ChangeLog entry that you added this annotation, and put /* __sigsuspend handles cancellation */ immediately above the LIBC_CANCEL_HANDLED line, following the pattern of all the other uses of LIBC_CANCEL_HANDLED; that'll make things at least a _little_ easier for future archaeologists. zw
On 05/11/2017 19:27, Zack Weinberg wrote: > On Fri, Nov 3, 2017 at 5:40 PM, Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> This patch simplify sigpause by remobing the single thread optimization >> since it will be handled already by the __sigsuspend call. >> >> Checked on x86_64-linux-gnu. >> >> * sysdeps/posix/sigpause.c (do_sigpause): Remove. >> (__sigpause): Rely on __sigsuspend to implement single thread >> optimization. > > LGTM except that the addition of LIBC_CANCEL_HANDLED sent me down a > rabbit hole - I might have to write a replacement for > tst-cancel-wrappers.sh that doesn't depend on these magic markers - > anyway, please mention in the ChangeLog entry that you added this > annotation, and put /* __sigsuspend handles cancellation */ > immediately above the LIBC_CANCEL_HANDLED line, following the pattern > of all the other uses of LIBC_CANCEL_HANDLED; that'll make things at > least a _little_ easier for future archaeologists. > > zw > In fact with my cancellation refactor patch my idea is just remove this fragile test which require this extra marks (since now we have now coverage for pretty much all symbols with runtime cancellation tests).
On Mon, Nov 6, 2017 at 6:09 AM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > On 05/11/2017 19:27, Zack Weinberg wrote: >> - I might have to write a replacement for >> tst-cancel-wrappers.sh that doesn't depend on these magic markers - > > In fact with my cancellation refactor patch my idea is just remove this > fragile test which require this extra marks (since now we have now > coverage for pretty much all symbols with runtime cancellation tests). I'm glad to hear this is already being addressed :) I haven't looked at your cancellation refactor patches because I don't know the guts of NPTL very well, unfortunately. zw
diff --git a/sysdeps/posix/sigpause.c b/sysdeps/posix/sigpause.c index 9038ed3..706195c 100644 --- a/sysdeps/posix/sigpause.c +++ b/sysdeps/posix/sigpause.c @@ -19,15 +19,13 @@ #include <errno.h> #include <signal.h> #include <stddef.h> /* For NULL. */ -#include <sysdep-cancel.h> #undef sigpause #include <sigset-cvt-mask.h> +#include <sysdep-cancel.h> -/* Set the mask of blocked signals to MASK, - wait for a signal to arrive, and then restore the mask. */ -static int -do_sigpause (int sig_or_mask, int is_sig) +int +__sigpause (int sig_or_mask, int is_sig) { sigset_t set; @@ -46,21 +44,6 @@ do_sigpause (int sig_or_mask, int is_sig) to do anything here. */ return __sigsuspend (&set); } - -int -__sigpause (int sig_or_mask, int is_sig) -{ - if (SINGLE_THREAD_P) - return do_sigpause (sig_or_mask, is_sig); - - int oldtype = LIBC_CANCEL_ASYNC (); - - int result = do_sigpause (sig_or_mask, is_sig); - - LIBC_CANCEL_RESET (oldtype); - - return result; -} libc_hidden_def (__sigpause) /* We have to provide a default version of this function since the @@ -87,3 +70,5 @@ __xpg_sigpause (int sig) return __sigpause (sig, 1); } strong_alias (__xpg_sigpause, __libc___xpg_sigpause) + +LIBC_CANCEL_HANDLED ();
This patch simplify sigpause by remobing the single thread optimization since it will be handled already by the __sigsuspend call. Checked on x86_64-linux-gnu. * sysdeps/posix/sigpause.c (do_sigpause): Remove. (__sigpause): Rely on __sigsuspend to implement single thread optimization. Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> --- ChangeLog | 4 ++++ sysdeps/posix/sigpause.c | 25 +++++-------------------- 2 files changed, 9 insertions(+), 20 deletions(-) -- 2.7.4