Re: [patch V5 09/26] posix-timers: Make signal overrun accounting sensible

[Thomas Gleixner]

On Mon, Oct 21 2024 at 23:54, Frederic Weisbecker wrote:
> Le Tue, Oct 01, 2024 at 10:42:12AM +0200, Thomas Gleixner a écrit :
>> +	/*
>> +	 * Set the overrun count to zero unconditionally. The posix timer
>> +	 * code does not self rearm periodic timers. They are rearmed from
>> +	 * dequeue_signal().
>> +	 *
>> +	 * But there is a situation where @q is already enqueued:
>> +	 *
>> +	 * 1) timer_settime()
>> +	 *      arm_timer()
>> +	 * 2) timer_expires()
>> +	 *      send_sigqueue(@q)
>> +	 *        enqueue(@q)
>> +	 * 3) timer_settime()
>> +	 *      arm_timer()
>> +	 * 4) timer_expires()
>> +	 *      send_sigqueue(@q) <- Observes @q already queued
>> +	 *
>> +	 * In this case incrementing si_overrun does not make sense because
>> +	 * there is no relationship between timer_settime() #1 and #2.
>> +	 *
>> +	 * The POSIX specification is useful as always: "The effect of
>> +	 * disarming or resetting a timer with pending expiration
>> +	 * notifications is unspecified."
>> +	 *
>> +	 * Just do the sensible thing and reset the overrun.
>> +	 */
>> +	q->info.si_overrun = 0;
>
> So this means that in the above example case, no signal at all is going to be
> delivered (because the seq will be impaired on the previously queued
> signal) and no overrun count will be incremented either?

So #2 queues the signal, but before delivery the timer is rearmed, which
invalidates the signal via the sequence count. So #4 has to set the
overrun counter which might be set already.

>> +
>>  	ret = 1; /* the signal is ignored */
>>  	result = TRACE_SIGNAL_IGNORED;
>>  	if (!prepare_signal(sig, t, false))
>> @@ -1968,15 +1996,9 @@ int send_sigqueue(struct sigqueue *q, st
>>  
>>  	ret = 0;
>>  	if (unlikely(!list_empty(&q->list))) {
>> -		/*
>> -		 * If an SI_TIMER entry is already queue just increment
>> -		 * the overrun count.
>> -		 */
>> -		q->info.si_overrun++;
>
> Who is ever incrementing this after that? I'm a bit confused between the
> timer overrun and the sigqueue overrun. Those seem to be two different
> things without any link...

Hmm. You're right. This should now never happen. Let me stare at it some
moar.

Thanks,

        tglx