Re: [patch v6 17/20] signal: Queue ignored posixtimers on ignore list

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, Nov 01 2024 at 21:47, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 15:21, Frederic Weisbecker wrote:
>> Le Thu, Oct 31, 2024 at 04:46:43PM +0100, Thomas Gleixner a écrit :
>>> +static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q)
>>> +{
>>> +	if (likely(!(q->flags & SIGQUEUE_PREALLOC) || q->info.si_code != SI_TIMER))
>>> +		__sigqueue_free(q);
>>> +	else
>>> +		posixtimer_sig_ignore(tsk, q);
>>
>> So this happens when the signal is ignored and delays it to when it will be
>> unignored. But the comment on do_sigaction() says:
>>
>> 		/*
>> 		 * POSIX 3.3.1.3:
>> 		 *  "Setting a signal action to SIG_IGN for a signal that is
>> 		 *   pending shall cause the pending signal to be discarded,
>> 		 *   whether or not it is blocked."
>> 		 *
>> 		 */
>>
>> Are posix timers an exception to that rule?
>>
>> Also I see flush_sigqueue_mask() called on other occasions, for example
>> when a STOP signal happens to remove pending CONT, not sure if posix
>> timers can set SIGCONT...
>
> No. The problem with posix timers is that they are magically different
> from regular signals in the case of periodic timers.
>
> When the signal is ignored at expiry, then the signal is not delivered
> and is 'dropped'. But when SIG_IGN is removed then the following period
> expiry has to deliver the signal.
>
> Right now the kernel ensures that by keeping the timer self rearming and
> rate limiting it for obvious reasons. That's a completely pointless
> exercise up to the point where SIG_IGN is removed.
>
> The only way to avoid the self rearming is to actually stop the timer
> when the signal is ignored and rearm it when SIG_IGN for the specific
> signal is removed.
>
> That's what this magic does...

But it does not exclude oneshot timers from this. Their signal has to be
dropped for real.

Delta patch below.

Thanks,

        tglx
---
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -731,7 +731,7 @@ void signal_wake_up_state(struct task_st
 		kick_process(t);
 }
 
-static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q);
+static inline bool posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q);
 
 static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q)
 {
@@ -1999,8 +1999,8 @@ int posixtimer_send_sigqueue(struct k_it
 			goto out;
 
 		if (hlist_unhashed(&tmr->ignored_list)) {
-			hlist_add_head(&tmr->ignored_list, &t->signal->ignored_posix_timers);
-			posixtimer_sigqueue_getref(q);
+			if (posixtimer_sig_ignore(t, tmr))
+				posixtimer_sigqueue_getref(q);
 		}
 		goto out;
 	}
@@ -2024,11 +2024,15 @@ int posixtimer_send_sigqueue(struct k_it
 	return 0;
 }
 
-static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q)
+static inline bool posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q)
 {
 	struct k_itimer *tmr = container_of(q, struct k_itimer, sigq);
 
+	/* Only queue periodic timer signals */
+	if (tmr->status != POSIX_TIMER_REQUEUE_PENDING)
+		return false;
 	hlist_add_head(&tmr->ignored_list, &tsk->signal->ignored_posix_timers);
+	return true;
 }
 
 static void posixtimer_sig_unignore(struct task_struct *tsk, int sig)