Make the issues vs. SIG_IGN understandable and remove the 15 years old
promise that a proper solution is already on the horizon.
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/time/posix-timers.c | 56 +++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 29 deletions(-)
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -325,11 +325,11 @@ int posix_timer_event(struct k_itimer *t
}
/*
- * This function gets called when a POSIX.1b interval timer expires. It
- * is used as a callback from the kernel internal timer. The
- * run_timer_list code ALWAYS calls with interrupts on.
-
- * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
+ * This function gets called when a POSIX.1b interval timer expires from
+ * the HRTIMER soft interrupt with interrupts enabled.
+ *
+ * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
+ * based timers.
*/
static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
{
@@ -347,9 +347,10 @@ static enum hrtimer_restart posix_timer_
if (posix_timer_event(timr, si_private)) {
/*
- * signal was not sent because of sig_ignor
- * we will not get a call back to restart it AND
- * it should be restarted.
+ * The signal was not queued due to SIG_IGN. As a
+ * consequence the timer is not going to be rearmed from
+ * the signal delivery path. But as a real signal handler
+ * can be installed later the timer must be rearmed here.
*/
if (timr->it_interval != 0) {
ktime_t now = hrtimer_cb_get_time(timer);
@@ -358,34 +359,31 @@ static enum hrtimer_restart posix_timer_
* FIXME: What we really want, is to stop this
* timer completely and restart it in case the
* SIG_IGN is removed. This is a non trivial
- * change which involves sighand locking
- * (sigh !), which we don't want to do late in
- * the release cycle.
+ * change to the signal handling code.
+ *
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie to avoid softirq
+ * starvation in case of SIG_IGN and a very small
+ * interval, which would put the timer right back
+ * on the softirq pending list. Moving now ahead of
+ * time tricks hrtimer_forward() to expire the
+ * timer later, while it still maintains the
+ * overrun accuracy for the price of a slightly
+ * inconsistency in the timer_gettime() case. This
+ * is at least better than a starved softirq.
*
- * For now we just let timers with an interval
- * less than a jiffie expire every jiffie to
- * avoid softirq starvation in case of SIG_IGN
- * and a very small interval, which would put
- * the timer right back on the softirq pending
- * list. By moving now ahead of time we trick
- * hrtimer_forward() to expire the timer
- * later, while we still maintain the overrun
- * accuracy, but have some inconsistency in
- * the timer_gettime() case. This is at least
- * better than a starved softirq. A more
- * complex fix which solves also another related
- * inconsistency is already in the pipeline.
+ * Only required when high resolution timers are
+ * enabled as the periodic tick based timers are
+ * automatically aligned to the next tick.
*/
-#ifdef CONFIG_HIGH_RES_TIMERS
- {
+ if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
ktime_t kj = NSEC_PER_SEC / HZ;
if (timr->it_interval < kj)
now = ktime_add(now, kj);
}
-#endif
- timr->it_overrun += hrtimer_forward(timer, now,
- timr->it_interval);
+
+ timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
ret = HRTIMER_RESTART;
++timr->it_requeue_pending;
timr->it_active = 1;
On Tue, Apr 25, 2023 at 08:49:24PM +0200, Thomas Gleixner wrote:
> Make the issues vs. SIG_IGN understandable and remove the 15 years old
> promise that a proper solution is already on the horizon.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> kernel/time/posix-timers.c | 56 +++++++++++++++++++++------------------------
> 1 file changed, 27 insertions(+), 29 deletions(-)
>
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -325,11 +325,11 @@ int posix_timer_event(struct k_itimer *t
> }
>
> /*
> - * This function gets called when a POSIX.1b interval timer expires. It
> - * is used as a callback from the kernel internal timer. The
> - * run_timer_list code ALWAYS calls with interrupts on.
> -
> - * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
> + * This function gets called when a POSIX.1b interval timer expires from
> + * the HRTIMER soft interrupt with interrupts enabled.
BTW, what arranges for this to be called in softirq with interrupts enabled?
The modes I see used here are HRTIMER_MODE_ABS or HRTIMER_MODE_REL and not
their _SOFT counterparts.
> + *
> + * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
> + * based timers.
> */
> static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
> {
> @@ -358,34 +359,31 @@ static enum hrtimer_restart posix_timer_
> * FIXME: What we really want, is to stop this
> * timer completely and restart it in case the
> * SIG_IGN is removed. This is a non trivial
> - * change which involves sighand locking
> - * (sigh !), which we don't want to do late in
> - * the release cycle.
> + * change to the signal handling code.
> + *
> + * For now let timers with an interval less than a
> + * jiffie expire every jiffie to avoid softirq
Or rather at least to the next jiffie, right? Because then in the next jiffie
it gets re-evaluated in case a real signal handler might have been set
in-between.
Or it could be:
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie (until a real sig handler
+ * is found set) to avoid softirq...
> + * starvation in case of SIG_IGN and a very small
> + * interval, which would put the timer right back
> + * on the softirq pending list. Moving now ahead of
> + * time tricks hrtimer_forward() to expire the
> + * timer later, while it still maintains the
> + * overrun accuracy for the price of a slightly
> + * inconsistency in the timer_gettime() case. This
> + * is at least better than a starved softirq.
[...]
> */
> -#ifdef CONFIG_HIGH_RES_TIMERS
> - {
> + if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
> ktime_t kj = NSEC_PER_SEC / HZ;
Could be TICK_NSECS?
Thanks!
>
> if (timr->it_interval < kj)
> now = ktime_add(now, kj);
> }
> -#endif
> - timr->it_overrun += hrtimer_forward(timer, now,
> - timr->it_interval);
> +
> + timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
> ret = HRTIMER_RESTART;
> ++timr->it_requeue_pending;
> timr->it_active = 1;
>
On Thu, Jun 01 2023 at 15:21, Frederic Weisbecker wrote:
> On Tue, Apr 25, 2023 at 08:49:24PM +0200, Thomas Gleixner wrote:
>> Make the issues vs. SIG_IGN understandable and remove the 15 years old
>> promise that a proper solution is already on the horizon.
>>
>> Signed-off-by: Thomas Gleixner <[email protected]>
>> ---
>> kernel/time/posix-timers.c | 56 +++++++++++++++++++++------------------------
>> 1 file changed, 27 insertions(+), 29 deletions(-)
>>
>> --- a/kernel/time/posix-timers.c
>> +++ b/kernel/time/posix-timers.c
>> @@ -325,11 +325,11 @@ int posix_timer_event(struct k_itimer *t
>> }
>>
>> /*
>> - * This function gets called when a POSIX.1b interval timer expires. It
>> - * is used as a callback from the kernel internal timer. The
>> - * run_timer_list code ALWAYS calls with interrupts on.
>> -
>> - * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
>> + * This function gets called when a POSIX.1b interval timer expires from
>> + * the HRTIMER soft interrupt with interrupts enabled.
>
> BTW, what arranges for this to be called in softirq with interrupts enabled?
> The modes I see used here are HRTIMER_MODE_ABS or HRTIMER_MODE_REL and not
> their _SOFT counterparts.
Duh. My RT biased brain tricked me.
>> + *
>> + * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
>> + * based timers.
>> */
>> static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
>> {
>> @@ -358,34 +359,31 @@ static enum hrtimer_restart posix_timer_
>> * FIXME: What we really want, is to stop this
>> * timer completely and restart it in case the
>> * SIG_IGN is removed. This is a non trivial
>> - * change which involves sighand locking
>> - * (sigh !), which we don't want to do late in
>> - * the release cycle.
>> + * change to the signal handling code.
>> + *
>> + * For now let timers with an interval less than a
>> + * jiffie expire every jiffie to avoid softirq
>
> Or rather at least to the next jiffie, right? Because then in the next jiffie
> it gets re-evaluated in case a real signal handler might have been set
> in-between.
>
> Or it could be:
>
> + * For now let timers with an interval less than a
> + * jiffie expire every jiffie (until a real sig handler
> + * is found set) to avoid softirq...
Let me rephrase that.
>> + * starvation in case of SIG_IGN and a very small
>> + * interval, which would put the timer right back
>> + * on the softirq pending list. Moving now ahead of
>> + * time tricks hrtimer_forward() to expire the
>> + * timer later, while it still maintains the
>> + * overrun accuracy for the price of a slightly
>> + * inconsistency in the timer_gettime() case. This
>> + * is at least better than a starved softirq.
> [...]
>> */
>> -#ifdef CONFIG_HIGH_RES_TIMERS
>> - {
>> + if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
>> ktime_t kj = NSEC_PER_SEC / HZ;
>
> Could be TICK_NSECS?
Yep. Fixed it up.
Make the issues vs. SIG_IGN understandable and remove the 15 years old
promise that a proper solution is already on the horizon.
Signed-off-by: Thomas Gleixner <[email protected]>
---
V2: Clarify comments and use TICK_NSECS - Frederic
---
kernel/time/posix-timers.c | 62 +++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 30 deletions(-)
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -326,11 +326,11 @@ int posix_timer_event(struct k_itimer *t
}
/*
- * This function gets called when a POSIX.1b interval timer expires. It
- * is used as a callback from the kernel internal timer. The
- * run_timer_list code ALWAYS calls with interrupts on.
-
- * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
+ * This function gets called when a POSIX.1b interval timer expires from
+ * the HRTIMER interrupt (soft interrupt on RT kernels).
+ *
+ * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
+ * based timers.
*/
static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
{
@@ -348,9 +348,10 @@ static enum hrtimer_restart posix_timer_
if (posix_timer_event(timr, si_private)) {
/*
- * signal was not sent because of sig_ignor
- * we will not get a call back to restart it AND
- * it should be restarted.
+ * The signal was not queued due to SIG_IGN. As a
+ * consequence the timer is not going to be rearmed from
+ * the signal delivery path. But as a real signal handler
+ * can be installed later the timer must be rearmed here.
*/
if (timr->it_interval != 0) {
ktime_t now = hrtimer_cb_get_time(timer);
@@ -359,34 +360,35 @@ static enum hrtimer_restart posix_timer_
* FIXME: What we really want, is to stop this
* timer completely and restart it in case the
* SIG_IGN is removed. This is a non trivial
- * change which involves sighand locking
- * (sigh !), which we don't want to do late in
- * the release cycle.
+ * change to the signal handling code.
+ *
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie and recheck for a
+ * valid signal handler.
+ *
+ * This avoids interrupt starvation in case of a
+ * very small interval, which would expire the
+ * timer immediately again.
*
- * For now we just let timers with an interval
- * less than a jiffie expire every jiffie to
- * avoid softirq starvation in case of SIG_IGN
- * and a very small interval, which would put
- * the timer right back on the softirq pending
- * list. By moving now ahead of time we trick
- * hrtimer_forward() to expire the timer
- * later, while we still maintain the overrun
- * accuracy, but have some inconsistency in
- * the timer_gettime() case. This is at least
- * better than a starved softirq. A more
- * complex fix which solves also another related
- * inconsistency is already in the pipeline.
+ * Moving now ahead of time by one jiffie tricks
+ * hrtimer_forward() to expire the timer later,
+ * while it still maintains the overrun accuracy
+ * for the price of a slight inconsistency in the
+ * timer_gettime() case. This is at least better
+ * than a starved softirq.
+ *
+ * Only required when high resolution timers are
+ * enabled as the periodic tick based timers are
+ * automatically aligned to the next tick.
*/
-#ifdef CONFIG_HIGH_RES_TIMERS
- {
- ktime_t kj = NSEC_PER_SEC / HZ;
+ if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
+ ktime_t kj = TICK_NSECS;
if (timr->it_interval < kj)
now = ktime_add(now, kj);
}
-#endif
- timr->it_overrun += hrtimer_forward(timer, now,
- timr->it_interval);
+
+ timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
ret = HRTIMER_RESTART;
++timr->it_requeue_pending;
timr->it_active = 1;
Le Thu, Jun 01, 2023 at 09:07:37PM +0200, Thomas Gleixner a ?crit :
> @@ -359,34 +360,35 @@ static enum hrtimer_restart posix_timer_
> * FIXME: What we really want, is to stop this
> * timer completely and restart it in case the
> * SIG_IGN is removed. This is a non trivial
> - * change which involves sighand locking
> - * (sigh !), which we don't want to do late in
> - * the release cycle.
> + * change to the signal handling code.
> + *
> + * For now let timers with an interval less than a
> + * jiffie expire every jiffie and recheck for a
> + * valid signal handler.
> + *
> + * This avoids interrupt starvation in case of a
> + * very small interval, which would expire the
> + * timer immediately again.
> *
> - * For now we just let timers with an interval
> - * less than a jiffie expire every jiffie to
> - * avoid softirq starvation in case of SIG_IGN
> - * and a very small interval, which would put
> - * the timer right back on the softirq pending
> - * list. By moving now ahead of time we trick
> - * hrtimer_forward() to expire the timer
> - * later, while we still maintain the overrun
> - * accuracy, but have some inconsistency in
> - * the timer_gettime() case. This is at least
> - * better than a starved softirq. A more
> - * complex fix which solves also another related
> - * inconsistency is already in the pipeline.
> + * Moving now ahead of time by one jiffie tricks
> + * hrtimer_forward() to expire the timer later,
> + * while it still maintains the overrun accuracy
> + * for the price of a slight inconsistency in the
> + * timer_gettime() case. This is at least better
> + * than a starved softirq.
Could be hardirq. How about:
"This is at least better than a timer storm."
Reviewed-by: Frederic Weisbecker <[email protected]>
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 63dede13d09850a8ace210f8e4227ac5a6b309ae
Gitweb: https://git.kernel.org/tip/63dede13d09850a8ace210f8e4227ac5a6b309ae
Author: Thomas Gleixner <[email protected]>
AuthorDate: Thu, 01 Jun 2023 21:07:37 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 05 Jun 2023 17:03:38 +02:00
posix-timers: Clarify posix_timer_fn() comments
Make the issues vs. SIG_IGN understandable and remove the 15 years old
promise that a proper solution is already on the horizon.
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/874jnrdmrq.ffs@tglx
---
kernel/time/posix-timers.c | 62 +++++++++++++++++++------------------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index f1a7c62..a22c183 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -326,11 +326,11 @@ int posix_timer_event(struct k_itimer *timr, int si_private)
}
/*
- * This function gets called when a POSIX.1b interval timer expires. It
- * is used as a callback from the kernel internal timer. The
- * run_timer_list code ALWAYS calls with interrupts on.
-
- * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
+ * This function gets called when a POSIX.1b interval timer expires from
+ * the HRTIMER interrupt (soft interrupt on RT kernels).
+ *
+ * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
+ * based timers.
*/
static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
{
@@ -348,9 +348,10 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
if (posix_timer_event(timr, si_private)) {
/*
- * signal was not sent because of sig_ignor
- * we will not get a call back to restart it AND
- * it should be restarted.
+ * The signal was not queued due to SIG_IGN. As a
+ * consequence the timer is not going to be rearmed from
+ * the signal delivery path. But as a real signal handler
+ * can be installed later the timer must be rearmed here.
*/
if (timr->it_interval != 0) {
ktime_t now = hrtimer_cb_get_time(timer);
@@ -359,34 +360,35 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
* FIXME: What we really want, is to stop this
* timer completely and restart it in case the
* SIG_IGN is removed. This is a non trivial
- * change which involves sighand locking
- * (sigh !), which we don't want to do late in
- * the release cycle.
+ * change to the signal handling code.
+ *
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie and recheck for a
+ * valid signal handler.
+ *
+ * This avoids interrupt starvation in case of a
+ * very small interval, which would expire the
+ * timer immediately again.
*
- * For now we just let timers with an interval
- * less than a jiffie expire every jiffie to
- * avoid softirq starvation in case of SIG_IGN
- * and a very small interval, which would put
- * the timer right back on the softirq pending
- * list. By moving now ahead of time we trick
- * hrtimer_forward() to expire the timer
- * later, while we still maintain the overrun
- * accuracy, but have some inconsistency in
- * the timer_gettime() case. This is at least
- * better than a starved softirq. A more
- * complex fix which solves also another related
- * inconsistency is already in the pipeline.
+ * Moving now ahead of time by one jiffie tricks
+ * hrtimer_forward() to expire the timer later,
+ * while it still maintains the overrun accuracy
+ * for the price of a slight inconsistency in the
+ * timer_gettime() case. This is at least better
+ * than a timer storm.
+ *
+ * Only required when high resolution timers are
+ * enabled as the periodic tick based timers are
+ * automatically aligned to the next tick.
*/
-#ifdef CONFIG_HIGH_RES_TIMERS
- {
- ktime_t kj = NSEC_PER_SEC / HZ;
+ if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
+ ktime_t kj = TICK_NSECS;
if (timr->it_interval < kj)
now = ktime_add(now, kj);
}
-#endif
- timr->it_overrun += hrtimer_forward(timer, now,
- timr->it_interval);
+
+ timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
ret = HRTIMER_RESTART;
++timr->it_requeue_pending;
timr->it_active = 1;
The following commit has been merged into the timers/core branch of tip:
Commit-ID: c78f261e5dcb415b9e35a13876fbf7d5f134c810
Gitweb: https://git.kernel.org/tip/c78f261e5dcb415b9e35a13876fbf7d5f134c810
Author: Thomas Gleixner <[email protected]>
AuthorDate: Thu, 01 Jun 2023 21:07:37 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 06 Jun 2023 00:12:55 +02:00
posix-timers: Clarify posix_timer_fn() comments
Make the issues vs. SIG_IGN understandable and remove the 15 years old
promise that a proper solution is already on the horizon.
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/874jnrdmrq.ffs@tglx
---
kernel/time/posix-timers.c | 62 +++++++++++++++++++------------------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index f1a7c62..a942020 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -326,11 +326,11 @@ int posix_timer_event(struct k_itimer *timr, int si_private)
}
/*
- * This function gets called when a POSIX.1b interval timer expires. It
- * is used as a callback from the kernel internal timer. The
- * run_timer_list code ALWAYS calls with interrupts on.
-
- * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
+ * This function gets called when a POSIX.1b interval timer expires from
+ * the HRTIMER interrupt (soft interrupt on RT kernels).
+ *
+ * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
+ * based timers.
*/
static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
{
@@ -348,9 +348,10 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
if (posix_timer_event(timr, si_private)) {
/*
- * signal was not sent because of sig_ignor
- * we will not get a call back to restart it AND
- * it should be restarted.
+ * The signal was not queued due to SIG_IGN. As a
+ * consequence the timer is not going to be rearmed from
+ * the signal delivery path. But as a real signal handler
+ * can be installed later the timer must be rearmed here.
*/
if (timr->it_interval != 0) {
ktime_t now = hrtimer_cb_get_time(timer);
@@ -359,34 +360,35 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
* FIXME: What we really want, is to stop this
* timer completely and restart it in case the
* SIG_IGN is removed. This is a non trivial
- * change which involves sighand locking
- * (sigh !), which we don't want to do late in
- * the release cycle.
+ * change to the signal handling code.
+ *
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie and recheck for a
+ * valid signal handler.
+ *
+ * This avoids interrupt starvation in case of a
+ * very small interval, which would expire the
+ * timer immediately again.
*
- * For now we just let timers with an interval
- * less than a jiffie expire every jiffie to
- * avoid softirq starvation in case of SIG_IGN
- * and a very small interval, which would put
- * the timer right back on the softirq pending
- * list. By moving now ahead of time we trick
- * hrtimer_forward() to expire the timer
- * later, while we still maintain the overrun
- * accuracy, but have some inconsistency in
- * the timer_gettime() case. This is at least
- * better than a starved softirq. A more
- * complex fix which solves also another related
- * inconsistency is already in the pipeline.
+ * Moving now ahead of time by one jiffie tricks
+ * hrtimer_forward() to expire the timer later,
+ * while it still maintains the overrun accuracy
+ * for the price of a slight inconsistency in the
+ * timer_gettime() case. This is at least better
+ * than a timer storm.
+ *
+ * Only required when high resolution timers are
+ * enabled as the periodic tick based timers are
+ * automatically aligned to the next tick.
*/
-#ifdef CONFIG_HIGH_RES_TIMERS
- {
- ktime_t kj = NSEC_PER_SEC / HZ;
+ if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
+ ktime_t kj = TICK_NSEC;
if (timr->it_interval < kj)
now = ktime_add(now, kj);
}
-#endif
- timr->it_overrun += hrtimer_forward(timer, now,
- timr->it_interval);
+
+ timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
ret = HRTIMER_RESTART;
++timr->it_requeue_pending;
timr->it_active = 1;
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 84999b8bdb4969816c7bb7c14c3a55ed42aa4b94
Gitweb: https://git.kernel.org/tip/84999b8bdb4969816c7bb7c14c3a55ed42aa4b94
Author: Thomas Gleixner <[email protected]>
AuthorDate: Thu, 01 Jun 2023 21:07:37 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Sun, 18 Jun 2023 22:41:52 +02:00
posix-timers: Clarify posix_timer_fn() comments
Make the issues vs. SIG_IGN understandable and remove the 15 years old
promise that a proper solution is already on the horizon.
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/874jnrdmrq.ffs@tglx
---
kernel/time/posix-timers.c | 62 +++++++++++++++++++------------------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index d8d2169..ae8799e 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -326,11 +326,11 @@ int posix_timer_event(struct k_itimer *timr, int si_private)
}
/*
- * This function gets called when a POSIX.1b interval timer expires. It
- * is used as a callback from the kernel internal timer. The
- * run_timer_list code ALWAYS calls with interrupts on.
-
- * This code is for CLOCK_REALTIME* and CLOCK_MONOTONIC* timers.
+ * This function gets called when a POSIX.1b interval timer expires from
+ * the HRTIMER interrupt (soft interrupt on RT kernels).
+ *
+ * Handles CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_BOOTTIME and CLOCK_TAI
+ * based timers.
*/
static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
{
@@ -348,9 +348,10 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
if (posix_timer_event(timr, si_private)) {
/*
- * signal was not sent because of sig_ignor
- * we will not get a call back to restart it AND
- * it should be restarted.
+ * The signal was not queued due to SIG_IGN. As a
+ * consequence the timer is not going to be rearmed from
+ * the signal delivery path. But as a real signal handler
+ * can be installed later the timer must be rearmed here.
*/
if (timr->it_interval != 0) {
ktime_t now = hrtimer_cb_get_time(timer);
@@ -359,34 +360,35 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
* FIXME: What we really want, is to stop this
* timer completely and restart it in case the
* SIG_IGN is removed. This is a non trivial
- * change which involves sighand locking
- * (sigh !), which we don't want to do late in
- * the release cycle.
+ * change to the signal handling code.
+ *
+ * For now let timers with an interval less than a
+ * jiffie expire every jiffie and recheck for a
+ * valid signal handler.
+ *
+ * This avoids interrupt starvation in case of a
+ * very small interval, which would expire the
+ * timer immediately again.
*
- * For now we just let timers with an interval
- * less than a jiffie expire every jiffie to
- * avoid softirq starvation in case of SIG_IGN
- * and a very small interval, which would put
- * the timer right back on the softirq pending
- * list. By moving now ahead of time we trick
- * hrtimer_forward() to expire the timer
- * later, while we still maintain the overrun
- * accuracy, but have some inconsistency in
- * the timer_gettime() case. This is at least
- * better than a starved softirq. A more
- * complex fix which solves also another related
- * inconsistency is already in the pipeline.
+ * Moving now ahead of time by one jiffie tricks
+ * hrtimer_forward() to expire the timer later,
+ * while it still maintains the overrun accuracy
+ * for the price of a slight inconsistency in the
+ * timer_gettime() case. This is at least better
+ * than a timer storm.
+ *
+ * Only required when high resolution timers are
+ * enabled as the periodic tick based timers are
+ * automatically aligned to the next tick.
*/
-#ifdef CONFIG_HIGH_RES_TIMERS
- {
- ktime_t kj = NSEC_PER_SEC / HZ;
+ if (IS_ENABLED(CONFIG_HIGHRES_TIMERS)) {
+ ktime_t kj = TICK_NSEC;
if (timr->it_interval < kj)
now = ktime_add(now, kj);
}
-#endif
- timr->it_overrun += hrtimer_forward(timer, now,
- timr->it_interval);
+
+ timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval);
ret = HRTIMER_RESTART;
++timr->it_requeue_pending;
timr->it_active = 1;