Re: [PATCH v9 22/32] timers: Keep the pinned timers separate from the others

[Sebastian Siewior]

On 2023-12-01 10:26:44 [+0100], Anna-Maria Behnsen wrote:
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1985,10 +1998,31 @@ static inline u64 __get_next_timer_interrupt(unsigned long basej, u64 basem,
>  		return expires;
>  	}
>  
> -	raw_spin_lock(&base->lock);
> -	nextevt = next_timer_interrupt(base, basej);
> +	base_local = this_cpu_ptr(&timer_bases[BASE_LOCAL]);
> +	base_global = this_cpu_ptr(&timer_bases[BASE_GLOBAL]);
> +
> +	raw_spin_lock(&base_local->lock);
> +	raw_spin_lock_nested(&base_global->lock, SINGLE_DEPTH_NESTING);
> +
> +	nextevt_local = next_timer_interrupt(base_local, basej);
> +	nextevt_global = next_timer_interrupt(base_global, basej);
>  
> -	if (base->timers_pending) {
> +	/*
> +	 * Check whether the local event is expiring before or at the same
> +	 * time as the global event.
> +	 *
> +	 * Note, that nextevt_global and nextevt_local might be based on
> +	 * different base->clk values. So it's not guaranteed that
> +	 * comparing with empty bases results in a correct local_first.

This ends like an unsolved mystery case. Could you add why one should
not worry about an incorrect local_first?

But seriously, how far apart can they get and what difference does it
make?  At timer enqueue time clk equals jiffies. At this point one clk
base could be at jiffies and the other might be a few jiffies before
that.
The next event (as in next_expiry) should be valid for both compare
wise. Both must be larger than jiffies. The delta between jiffies and
next event has to be less than NEXT_TIMER_MAX_DELTA for each base.

> +	 */
> +	if (base_local->timers_pending && base_global->timers_pending)
> +		local_first = time_before_eq(nextevt_local, nextevt_global);
> +	else
> +		local_first = base_local->timers_pending;
> +
> +	nextevt = local_first ? nextevt_local : nextevt_global;
> +
> +	if (base_local->timers_pending || base_global->timers_pending) {
>  		/* If we missed a tick already, force 0 delta */
>  		if (time_before(nextevt, basej))
>  			nextevt = basej;

So if nextevt_local missed a tick and nextevt_global is
NEXT_TIMER_MAX_DELTA-1 (so we get the largest difference possible
between those two) then the time_before_eq() should still come out
right. We could still miss more than one tick.

This looks good. I just don't understand the (above) comment.

Sebastian