Re: [patch V5 08/26] posix-timers: Make signal delivery consistent

From: Thomas Gleixner
Date: Thu Oct 24 2024 - 04:52:46 EST


On Mon, Oct 21 2024 at 16:40, Frederic Weisbecker wrote:
> Le Tue, Oct 01, 2024 at 10:42:10AM +0200, Thomas Gleixner a écrit :
>> --- a/kernel/time/posix-timers.c
>> +++ b/kernel/time/posix-timers.c
>> @@ -269,7 +269,10 @@ bool posixtimer_deliver_signal(struct ke
>> if (!timr)
>> goto out;
>>
>> - if (timr->it_interval && timr->it_signal_seq == info->si_sys_private) {
>> + if (timr->it_signal_seq != info->si_sys_private)
>> + goto out_unlock;
>> +
>> + if (timr->it_interval && timr->it_status == POSIX_TIMER_REQUEUE_PENDING) {
>
> Can it be something else than POSIX_TIMER_REQUEUE_PENDING actually?
> And if not, should it be a WARN_ON() ?

Good point. It should not be anything else than pending.

>> timr->kclock->timer_rearm(timr);
>>
>> timr->it_status = POSIX_TIMER_ARMED;
>> @@ -281,6 +284,7 @@ bool posixtimer_deliver_signal(struct ke
>> }
>> ret = true;
>>
>> +out_unlock:
>> unlock_timer(timr, flags);
>> out:
>> spin_lock(&current->sighand->siglock);
>> @@ -293,19 +297,19 @@ bool posixtimer_deliver_signal(struct ke
>> int posix_timer_queue_signal(struct k_itimer *timr)
>> {
>> enum posix_timer_state state = POSIX_TIMER_DISARMED;
>> - int ret, si_private = 0;
>> enum pid_type type;
>> + int ret;
>>
>> lockdep_assert_held(&timr->it_lock);
>>
>> if (timr->it_interval) {
>> + timr->it_signal_seq++;
>
> Is the increment here is still needed then, since it's done
> from del and set?

Probably not. Let me stare at it.

Thanks,

tglx