Messages in this thread | | | From | Anna-Maria Behnsen <> | Subject | Re: [PATCH v10 13/20] timers: Add get next timer interrupt functionality for remote CPUs | Date | Mon, 19 Feb 2024 17:57:16 +0100 |
| |
Frederic Weisbecker <frederic@kernel.org> writes:
> Le Mon, Jan 15, 2024 at 03:37:36PM +0100, Anna-Maria Behnsen a écrit : >> +# ifdef CONFIG_SMP >> +/** >> + * fetch_next_timer_interrupt_remote() - Store next timers into @tevt >> + * @basej: base time jiffies >> + * @basem: base time clock monotonic >> + * @tevt: Pointer to the storage for the expiry values >> + * @cpu: Remote CPU >> + * >> + * Stores the next pending local and global timer expiry values in the >> + * struct pointed to by @tevt. If a queue is empty the corresponding >> + * field is set to KTIME_MAX. If local event expires before global >> + * event, global event is set to KTIME_MAX as well. >> + * >> + * Caller needs to make sure timer base locks are held (use >> + * timer_lock_remote_bases() for this purpose). >> + */ >> +void fetch_next_timer_interrupt_remote(unsigned long basej, u64 basem, >> + struct timer_events *tevt, >> + unsigned int cpu) >> +{ >> + struct timer_base *base_local, *base_global; >> + >> + /* Preset local / global events */ >> + tevt->local = tevt->global = KTIME_MAX; >> + >> + base_local = per_cpu_ptr(&timer_bases[BASE_LOCAL], cpu); >> + base_global = per_cpu_ptr(&timer_bases[BASE_GLOBAL], cpu); >> + >> + lockdep_assert_held(&base_local->lock); >> + lockdep_assert_held(&base_global->lock); >> + >> + fetch_next_timer_interrupt(basej, basem, base_local, base_global, tevt); > > If the next timer is global and it is <= jiffies + 1, the result will be > returned in tevt.local only and not on tevt.global. So a remote fetch may miss it.
Oh no. But yes, sounds reasonable.
> For this to work on both local and remote fetch, you may need: > > diff --git a/kernel/time/timer.c b/kernel/time/timer.c > index 320eb4ceafa2..64ce9a7760f5 100644 > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -2004,6 +2007,8 @@ static unsigned long fetch_next_timer_interrupt(unsigned long basej, u64 basem, > if (time_before(nextevt, basej)) > nextevt = basej; > tevt->local = basem + (u64)(nextevt - basej) * TICK_NSEC; > + if (!local_first) > + tevt->global = tevt->local; > return nextevt; > } >
Will fix it - with a big comment explaining why this is required for remote call sites and will not hurt when executed on the local cpu.
Thanks a lot!
| |