[patch V5 05/26] posix-timers: Drop signal if timer has been deleted or reprogrammed

Thomas Gleixner posted 26 patches 1 month, 4 weeks ago
There is a newer version of this series
[patch V5 05/26] posix-timers: Drop signal if timer has been deleted or reprogrammed
Posted by Thomas Gleixner 1 month, 4 weeks ago
From: Thomas Gleixner <tglx@linutronix.de>

No point in delivering a signal from the past. POSIX does not specify the
behaviour here:

 - "The effect of disarming or resetting a timer with pending expiration
    notifications is unspecified."

 - "The disposition of pending signals for the deleted timer is unspecified."

In both cases it is reasonable to expect that pending signals are
discarded. Especially in the reprogramming case it does not make sense to
account for previous overruns or to deliver a signal for a timer which has
been disarmed.

Drop the signal as that is conistent and understandable behaviour.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/time/posix-timers.c |  9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
---
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index d7ed7542f803..b5d7e71c10f2 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -250,14 +250,14 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
 }
 
 /*
- * This function is called from the signal delivery code if
- * info::si_sys_private is not zero, which indicates that the timer has to
- * be rearmed. Restart the timer and update info::si_overrun.
+ * This function is called from the signal delivery code. It decides
+ * whether the signal should be dropped and rearms interval timers.
  */
 bool posixtimer_deliver_signal(struct kernel_siginfo *info)
 {
 	struct k_itimer *timr;
 	unsigned long flags;
+	bool ret = false;
 
 	/*
 	 * Release siglock to ensure proper locking order versus
@@ -279,6 +279,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
 
 		info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);
 	}
+	ret = true;
 
 	unlock_timer(timr, flags);
 out:
@@ -286,7 +287,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
 
 	/* Don't expose the si_sys_private value to userspace */
 	info->si_sys_private = 0;
-	return true;
+	return ret;
 }
 
 int posix_timer_queue_signal(struct k_itimer *timr)
Re: [patch V5 05/26] posix-timers: Drop signal if timer has been deleted or reprogrammed
Posted by Frederic Weisbecker 1 month, 1 week ago
Le Tue, Oct 01, 2024 at 10:42:06AM +0200, Thomas Gleixner a écrit :
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> No point in delivering a signal from the past. POSIX does not specify the
> behaviour here:
> 
>  - "The effect of disarming or resetting a timer with pending expiration
>     notifications is unspecified."
> 
>  - "The disposition of pending signals for the deleted timer is unspecified."
> 
> In both cases it is reasonable to expect that pending signals are
> discarded. Especially in the reprogramming case it does not make sense to
> account for previous overruns or to deliver a signal for a timer which has
> been disarmed.

The change below indeed checks if the timer has been deleted but not if
it has been reprogrammed/disarmed/reset.

Or am I missing something?

Thanks.

> 
> Drop the signal as that is conistent and understandable behaviour.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/time/posix-timers.c |  9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> ---
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index d7ed7542f803..b5d7e71c10f2 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -250,14 +250,14 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
>  }
>  
>  /*
> - * This function is called from the signal delivery code if
> - * info::si_sys_private is not zero, which indicates that the timer has to
> - * be rearmed. Restart the timer and update info::si_overrun.
> + * This function is called from the signal delivery code. It decides
> + * whether the signal should be dropped and rearms interval timers.
>   */
>  bool posixtimer_deliver_signal(struct kernel_siginfo *info)
>  {
>  	struct k_itimer *timr;
>  	unsigned long flags;
> +	bool ret = false;
>  
>  	/*
>  	 * Release siglock to ensure proper locking order versus
> @@ -279,6 +279,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
>  
>  		info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);
>  	}
> +	ret = true;
>  
>  	unlock_timer(timr, flags);
>  out:
> @@ -286,7 +287,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
>  
>  	/* Don't expose the si_sys_private value to userspace */
>  	info->si_sys_private = 0;
> -	return true;
> +	return ret;
>  }
>  
>  int posix_timer_queue_signal(struct k_itimer *timr)
> 
Re: [patch V5 05/26] posix-timers: Drop signal if timer has been deleted or reprogrammed
Posted by Thomas Gleixner 1 month ago
On Mon, Oct 21 2024 at 14:29, Frederic Weisbecker wrote:
> Le Tue, Oct 01, 2024 at 10:42:06AM +0200, Thomas Gleixner a écrit :
>> No point in delivering a signal from the past. POSIX does not specify the
>> behaviour here:
>> 
>>  - "The effect of disarming or resetting a timer with pending expiration
>>     notifications is unspecified."
>> 
>>  - "The disposition of pending signals for the deleted timer is unspecified."
>> 
>> In both cases it is reasonable to expect that pending signals are
>> discarded. Especially in the reprogramming case it does not make sense to
>> account for previous overruns or to deliver a signal for a timer which has
>> been disarmed.
>
> The change below indeed checks if the timer has been deleted but not if
> it has been reprogrammed/disarmed/reset.
>
> Or am I missing something?

No. You are right. This is a change log left over from a previous
version. This rearm/disarm part is handled later in the series.

Thanks,

        tglx
Re: [patch V5 05/26] posix-timers: Drop signal if timer has been deleted or reprogrammed
Posted by Frederic Weisbecker 1 month ago
Le Thu, Oct 24, 2024 at 10:40:08AM +0200, Thomas Gleixner a écrit :
> On Mon, Oct 21 2024 at 14:29, Frederic Weisbecker wrote:
> > Le Tue, Oct 01, 2024 at 10:42:06AM +0200, Thomas Gleixner a écrit :
> >> No point in delivering a signal from the past. POSIX does not specify the
> >> behaviour here:
> >> 
> >>  - "The effect of disarming or resetting a timer with pending expiration
> >>     notifications is unspecified."
> >> 
> >>  - "The disposition of pending signals for the deleted timer is unspecified."
> >> 
> >> In both cases it is reasonable to expect that pending signals are
> >> discarded. Especially in the reprogramming case it does not make sense to
> >> account for previous overruns or to deliver a signal for a timer which has
> >> been disarmed.
> >
> > The change below indeed checks if the timer has been deleted but not if
> > it has been reprogrammed/disarmed/reset.
> >
> > Or am I missing something?
> 
> No. You are right. This is a change log left over from a previous
> version. This rearm/disarm part is handled later in the series.

Right, I realized that after. Anyway:

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>


> 
> Thanks,
> 
>         tglx
[tip: timers/core] posix-timers: Drop signal if timer has been deleted or reprogrammed
Posted by tip-bot2 for Thomas Gleixner 1 month ago
The following commit has been merged into the timers/core branch of tip:

Commit-ID:     2860d4d315dc01f001dfd328adaf2ab440c47dd3
Gitweb:        https://git.kernel.org/tip/2860d4d315dc01f001dfd328adaf2ab440c47dd3
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 01 Oct 2024 10:42:06 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 29 Oct 2024 11:43:19 +01:00

posix-timers: Drop signal if timer has been deleted or reprogrammed

No point in delivering a signal from the past. POSIX does not specify the
behaviour here:

 - "The effect of disarming or resetting a timer with pending expiration
    notifications is unspecified."

 - "The disposition of pending signals for the deleted timer is unspecified."

In both cases it is reasonable to expect that pending signals are
discarded. Especially in the reprogramming case it does not make sense to
account for previous overruns or to deliver a signal for a timer which has
been disarmed.

Drop the signal as that is conistent and understandable behaviour.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/all/20241001083835.553646280@linutronix.de

---
 kernel/time/posix-timers.c |  9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index dd0b1df..22e1d6b 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -250,14 +250,14 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
 }
 
 /*
- * This function is called from the signal delivery code if
- * info::si_sys_private is not zero, which indicates that the timer has to
- * be rearmed. Restart the timer and update info::si_overrun.
+ * This function is called from the signal delivery code. It decides
+ * whether the signal should be dropped and rearms interval timers.
  */
 bool posixtimer_deliver_signal(struct kernel_siginfo *info)
 {
 	struct k_itimer *timr;
 	unsigned long flags;
+	bool ret = false;
 
 	/*
 	 * Release siglock to ensure proper locking order versus
@@ -279,6 +279,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
 
 		info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);
 	}
+	ret = true;
 
 	unlock_timer(timr, flags);
 out:
@@ -286,7 +287,7 @@ out:
 
 	/* Don't expose the si_sys_private value to userspace */
 	info->si_sys_private = 0;
-	return true;
+	return ret;
 }
 
 int posix_timer_queue_signal(struct k_itimer *timr)