signal: avoid clearing TIF_SIGPENDING in recalc_sigpending() if unset

[PATCH] signal: avoid clearing TIF_SIGPENDING in recalc_sigpending() if unset

Posted by Mateusz Guzik 11 months, 1 week ago

Clearing is an atomic op and the flag is not set most of the time.

When creating and destroying threads in the same process with the
pthread family, the primary bottleneck is calls to sigprocmask which
take the process-wide sighand lock.

Avoiding the atomic gives me a 2% bump in start/teardown rate at 24-core
scale.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 kernel/signal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 081f19a24506..4727b108d842 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -176,9 +176,10 @@ static bool recalc_sigpending_tsk(struct task_struct *t)
 
 void recalc_sigpending(void)
 {
-	if (!recalc_sigpending_tsk(current) && !freezing(current))
-		clear_thread_flag(TIF_SIGPENDING);
-
+	if (!recalc_sigpending_tsk(current) && !freezing(current)) {
+		if (test_thread_flag(TIF_SIGPENDING))
+			clear_thread_flag(TIF_SIGPENDING);
+	}
 }
 EXPORT_SYMBOL(recalc_sigpending);
 
-- 
2.43.0

Re: [PATCH] signal: avoid clearing TIF_SIGPENDING in recalc_sigpending() if unset

Posted by Andrew Morton 11 months, 1 week ago

On Mon,  3 Mar 2025 14:49:08 +0100 Mateusz Guzik <mjguzik@gmail.com> wrote:

> Clearing is an atomic op and the flag is not set most of the time.
> 
> When creating and destroying threads in the same process with the
> pthread family, the primary bottleneck is calls to sigprocmask which
> take the process-wide sighand lock.
> 
> Avoiding the atomic gives me a 2% bump in start/teardown rate at 24-core
> scale.
> 

Neat.  Perhaps add an unlikely() also?

--- a/kernel/signal.c~signal-avoid-clearing-tif_sigpending-in-recalc_sigpending-if-unset-fix
+++ a/kernel/signal.c
@@ -177,7 +177,7 @@ static bool recalc_sigpending_tsk(struct
 void recalc_sigpending(void)
 {
 	if (!recalc_sigpending_tsk(current) && !freezing(current)) {
-		if (test_thread_flag(TIF_SIGPENDING))
+		if (unlikely(test_thread_flag(TIF_SIGPENDING)))
 			clear_thread_flag(TIF_SIGPENDING);
 	}
 }
_

Re: [PATCH] signal: avoid clearing TIF_SIGPENDING in recalc_sigpending() if unset

Posted by Mateusz Guzik 11 months, 1 week ago

On Mon, Mar 3, 2025 at 11:24 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon,  3 Mar 2025 14:49:08 +0100 Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> > Clearing is an atomic op and the flag is not set most of the time.
> >
> > When creating and destroying threads in the same process with the
> > pthread family, the primary bottleneck is calls to sigprocmask which
> > take the process-wide sighand lock.
> >
> > Avoiding the atomic gives me a 2% bump in start/teardown rate at 24-core
> > scale.
> >
>
> Neat.  Perhaps add an unlikely() also?
>
> --- a/kernel/signal.c~signal-avoid-clearing-tif_sigpending-in-recalc_sigpending-if-unset-fix
> +++ a/kernel/signal.c
> @@ -177,7 +177,7 @@ static bool recalc_sigpending_tsk(struct
>  void recalc_sigpending(void)
>  {
>         if (!recalc_sigpending_tsk(current) && !freezing(current)) {
> -               if (test_thread_flag(TIF_SIGPENDING))
> +               if (unlikely(test_thread_flag(TIF_SIGPENDING)))
>                         clear_thread_flag(TIF_SIGPENDING);
>         }
>  }
> _
>

Ye that makes sense, I don't think Oleg is going to object :)

Thanks for a quick turn around to you both,

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH] signal: avoid clearing TIF_SIGPENDING in recalc_sigpending() if unset

Posted by Oleg Nesterov 11 months, 1 week ago

On 03/03, Mateusz Guzik wrote:
>
>  void recalc_sigpending(void)
>  {
> -	if (!recalc_sigpending_tsk(current) && !freezing(current))
> -		clear_thread_flag(TIF_SIGPENDING);
> -
> +	if (!recalc_sigpending_tsk(current) && !freezing(current)) {
> +		if (test_thread_flag(TIF_SIGPENDING))
> +			clear_thread_flag(TIF_SIGPENDING);
> +	}

Acked-by: Oleg Nesterov <oleg@redhat.com>