diff mbox series

[v8,09/25] timer: Split out get next timer functionality

Message ID 20231004123454.15691-10-anna-maria@linutronix.de
State New
Headers
Series timer: Move from a push remote at enqueue to a pull at expiry model |

Commit Message

Anna-Maria Behnsen Oct. 4, 2023, 12:34 p.m. UTC 
  Split out get next timer functionality to make it reusable in other
places. Thereby the order of getting the next expiry, forwarding the base
clock and mark timer bases as idle, is changed. This change of order
shouldn't have any impact, as nothing inside the function relies on the
idle value or the updated timer base clock.

Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
---
 kernel/time/timer.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

Frederic Weisbecker Oct. 9, 2023, 9:15 p.m. UTC | #1 
Le Wed, Oct 04, 2023 at 02:34:38PM +0200, Anna-Maria Behnsen a écrit :
> Split out get next timer functionality to make it reusable in other
> places. Thereby the order of getting the next expiry, forwarding the base
> clock and mark timer bases as idle, is changed. This change of order
> shouldn't have any impact, as nothing inside the function relies on the
> idle value or the updated timer base clock.
> 
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> ---
>  kernel/time/timer.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 18f8aac9b19a..f443aa807fbc 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1911,6 +1911,24 @@ static u64 cmp_next_hrtimer_event(u64 basem, u64 expires)
>  	return DIV_ROUND_UP_ULL(nextevt, TICK_NSEC) * TICK_NSEC;
>  }
>  
> +static inline unsigned long __get_next_timer_interrupt(unsigned long basej,
> +						       struct timer_base *base)
> +{
> +	unsigned long nextevt;
> +
> +	if (base->next_expiry_recalc)
> +		next_expiry_recalc(base);
> +	nextevt = base->next_expiry;
> +
> +	if (base->timers_pending) {
> +		/* If we missed a tick already, force 0 delta */
> +		if (time_before(nextevt, basej))
> +			nextevt = basej;
> +	}
> +
> +	return nextevt;
> +}
> +
>  /**
>   * get_next_timer_interrupt - return the time (clock mono) of the next timer
>   * @basej:	base time jiffies
> @@ -1933,9 +1951,7 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
>  		return expires;
>  
>  	raw_spin_lock(&base->lock);
> -	if (base->next_expiry_recalc)
> -		next_expiry_recalc(base);
> -	nextevt = base->next_expiry;
> +	nextevt = __get_next_timer_interrupt(basej, base);
>  
>  	/*
>  	 * We have a fresh next event. Check whether we can forward the
> @@ -1952,14 +1968,10 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
>  	 */
>  	base->is_idle = time_after(nextevt, basej + 1);
>  
> -	if (base->timers_pending) {
> -		/* If we missed a tick already, force 0 delta */
> -		if (time_before(nextevt, basej))
> -			nextevt = basej;
> -		expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
> -	}
>  	raw_spin_unlock(&base->lock);
>  
> +	expires = basem + (u64)(nextevt - basej) * TICK_NSEC;

Does that compute KTIME_MAX when there is no timers pending?

Thanks.

> +
>  	return cmp_next_hrtimer_event(basem, expires);
>  }
>  
> -- 
> 2.39.2
>
Thomas Gleixner Oct. 9, 2023, 10:17 p.m. UTC | #2 
On Wed, Oct 04 2023 at 14:34, Anna-Maria Behnsen wrote:
> Split out get next timer functionality to make it reusable in other
> places. Thereby the order of getting the next expiry, forwarding the base
> clock and mark timer bases as idle, is changed. This change of order
> shouldn't have any impact, as nothing inside the function relies on the
> idle value or the updated timer base clock.

This interestingly corrects the issue of patch 8/25 because

> +	nextevt = __get_next_timer_interrupt(basej, base);
>  
>  	/*
>  	 * We have a fresh next event. Check whether we can forward the
> @@ -1952,14 +1968,10 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
>  	 */
>  	base->is_idle = time_after(nextevt, basej + 1);

base->is_idle is now handled in the correct order vs. the nextevt evaluation.
  
> -	if (base->timers_pending) {
> -		/* If we missed a tick already, force 0 delta */
> -		if (time_before(nextevt, basej))
> -			nextevt = basej;
> -		expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
> -	}
>  	raw_spin_unlock(&base->lock);

Thanks,

        tglx
Thomas Gleixner Oct. 9, 2023, 10:24 p.m. UTC | #3 
On Mon, Oct 09 2023 at 23:15, Frederic Weisbecker wrote:
> Le Wed, Oct 04, 2023 at 02:34:38PM +0200, Anna-Maria Behnsen a écrit :
>>  
>> +	expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
>
> Does that compute KTIME_MAX when there is no timers pending?

Probably not :)
diff mbox series

Patch

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 18f8aac9b19a..f443aa807fbc 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1911,6 +1911,24 @@  static u64 cmp_next_hrtimer_event(u64 basem, u64 expires)
 	return DIV_ROUND_UP_ULL(nextevt, TICK_NSEC) * TICK_NSEC;
 }
 
+static inline unsigned long __get_next_timer_interrupt(unsigned long basej,
+						       struct timer_base *base)
+{
+	unsigned long nextevt;
+
+	if (base->next_expiry_recalc)
+		next_expiry_recalc(base);
+	nextevt = base->next_expiry;
+
+	if (base->timers_pending) {
+		/* If we missed a tick already, force 0 delta */
+		if (time_before(nextevt, basej))
+			nextevt = basej;
+	}
+
+	return nextevt;
+}
+
 /**
  * get_next_timer_interrupt - return the time (clock mono) of the next timer
  * @basej:	base time jiffies
@@ -1933,9 +1951,7 @@  u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
 		return expires;
 
 	raw_spin_lock(&base->lock);
-	if (base->next_expiry_recalc)
-		next_expiry_recalc(base);
-	nextevt = base->next_expiry;
+	nextevt = __get_next_timer_interrupt(basej, base);
 
 	/*
 	 * We have a fresh next event. Check whether we can forward the
@@ -1952,14 +1968,10 @@  u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
 	 */
 	base->is_idle = time_after(nextevt, basej + 1);
 
-	if (base->timers_pending) {
-		/* If we missed a tick already, force 0 delta */
-		if (time_before(nextevt, basej))
-			nextevt = basej;
-		expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
-	}
 	raw_spin_unlock(&base->lock);
 
+	expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
+
 	return cmp_next_hrtimer_event(basem, expires);
 }