The recalc_sigpending_and_wake() function calls recalc_sigpending_tsk() and signal_wake_up(),
both of which set the TIF_SIGPENDING flag, so when recalc_sigpending_tsk() returns true,
the TIF_SIGPENDING flag will be set twice.
Maybe we can take away the set TIF_SIGPENDING logic from recalc_sigpending_tsk(),
so that recalc_sigpending_tsk() just determines whether the TIF_SIGPENDING flag needs to be set,
and the actual set TIF_SIGPENDING flag logic is executed outside.
kernel/signal.c:175: recalc_sigpending_and_wake()
kernel/signal.c:154: recalc_sigpending_tsk()
Signed-off-by: Hao Wu <[email protected]>
---
kernel/signal.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 9b04631acde8..6c5a00cd7e9a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -157,7 +157,6 @@ static bool recalc_sigpending_tsk(struct task_struct *t)
PENDING(&t->pending, &t->blocked) ||
PENDING(&t->signal->shared_pending, &t->blocked) ||
cgroup_task_frozen(t)) {
- set_tsk_thread_flag(t, TIF_SIGPENDING);
return true;
}
@@ -181,8 +180,11 @@ void recalc_sigpending_and_wake(struct task_struct *t)
void recalc_sigpending(void)
{
- if (!recalc_sigpending_tsk(current) && !freezing(current))
+ if (recalc_sigpending_tsk(current)) {
+ set_tsk_thread_flag(t, TIF_SIGPENDING);
+ } else if (!freezing(current)) {
clear_thread_flag(TIF_SIGPENDING);
+ }
}
EXPORT_SYMBOL(recalc_sigpending);
@@ -2325,7 +2327,9 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
* So check for any that we should take before resuming user mode.
* This sets TIF_SIGPENDING, but never clears it.
*/
- recalc_sigpending_tsk(current);
+ if (recalc_sigpending_tsk(current)) {
+ set_tsk_thread_flag(t, TIF_SIGPENDING);
+ }
}
static void ptrace_do_notify(int signr, int exit_code, int why)
2.32.0
Hao Wu <[email protected]> writes:
> The recalc_sigpending_and_wake() function calls recalc_sigpending_tsk() and signal_wake_up(),
> both of which set the TIF_SIGPENDING flag, so when recalc_sigpending_tsk() returns true,
> the TIF_SIGPENDING flag will be set twice.
>
> Maybe we can take away the set TIF_SIGPENDING logic from recalc_sigpending_tsk(),
> so that recalc_sigpending_tsk() just determines whether the TIF_SIGPENDING flag needs to be set,
> and the actual set TIF_SIGPENDING flag logic is executed outside.
>
> kernel/signal.c:175: recalc_sigpending_and_wake()
> kernel/signal.c:154: recalc_sigpending_tsk()
I just saw this.
I agree that it is unfortunate that recalc_sigpending_and_wake sets
TIF_SIGPENDING twice.
Looking at the code only force_sig_info_to_task calls
recalc_sigpending_and_wake. I have some work underway that removes the
need to call recalc_sigpending in force_sig_info_to_task so I believe
that will resolve the matter.
Regardless of changes I have planned force_sig_info_to_task appears to
be the proper place to resolve this issue. It looks like
complete_signal called from __send_signal_locked called from
send_signal_locked called from force_sig_info_to_task calls
signal_wake_up if that is needed at all. So I believe the issue you are
seeing could be solved be simply replacing recalc_sigpending_and_wake
with recalc_sigpending_tsk.
Some more analysis might reveal and even nicer clean up. If
sending the signal is always going to call signal_wake_up calling
recalc_sigpending_tsk might not even be necessary.
Good spotting of a rough spot in the code. I don't think your proposed
change is the way we should go about fixing it.
Eric
>
> Signed-off-by: Hao Wu <[email protected]>
> ---
> kernel/signal.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 9b04631acde8..6c5a00cd7e9a 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -157,7 +157,6 @@ static bool recalc_sigpending_tsk(struct task_struct *t)
> PENDING(&t->pending, &t->blocked) ||
> PENDING(&t->signal->shared_pending, &t->blocked) ||
> cgroup_task_frozen(t)) {
> - set_tsk_thread_flag(t, TIF_SIGPENDING);
> return true;
> }
> @@ -181,8 +180,11 @@ void recalc_sigpending_and_wake(struct task_struct *t)
> void recalc_sigpending(void)
> {
> - if (!recalc_sigpending_tsk(current) && !freezing(current))
> + if (recalc_sigpending_tsk(current)) {
> + set_tsk_thread_flag(t, TIF_SIGPENDING);
> + } else if (!freezing(current)) {
> clear_thread_flag(TIF_SIGPENDING);
> + }
> }
> EXPORT_SYMBOL(recalc_sigpending);
> @@ -2325,7 +2327,9 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
> * So check for any that we should take before resuming user mode.
> * This sets TIF_SIGPENDING, but never clears it.
> */
> - recalc_sigpending_tsk(current);
> + if (recalc_sigpending_tsk(current)) {
> + set_tsk_thread_flag(t, TIF_SIGPENDING);
> + }
> }
> static void ptrace_do_notify(int signr, int exit_code, int why)
> 2.32.0