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

[Frederic Weisbecker]

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() ?

>  		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?

Thanks.

>  		state = POSIX_TIMER_REQUEUE_PENDING;
> -		si_private = ++timr->it_signal_seq;
>  	}
>  	timr->it_status = state;
>  
>  	type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID;
> -	ret = send_sigqueue(timr->sigq, timr->it_pid, type, si_private);
> +	ret = send_sigqueue(timr->sigq, timr->it_pid, type, timr->it_signal_seq);
>  	/* If we failed to send the signal the timer stops. */
>  	return ret > 0;
>  }