Messages in this thread | | | Date | Wed, 10 May 2023 12:16:22 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH v6 15/21] timer: Add get next timer interrupt functionality for remote CPUs |
| |
Le Wed, May 10, 2023 at 09:28:11AM +0200, Anna-Maria Behnsen a écrit : > +/** > + * fetch_next_timer_interrupt_remote > + * @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). Caller must make sure > + * interrupts are reopened, if required. > + */ > +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(base_local, base_global, basej, basem, tevt); > + > + raw_spin_unlock(&base_global->lock); > + raw_spin_unlock(&base_local->lock);
Oh so that makes:
LOCK(baseL) LOCK(baseG) LOCK(tmc) UNLOCK(baseG) UNLOCK(baseL) UNLOCK(tmc)
I guess you can keep the bases locks locked until the end of tmigr_handle_remote_cpu(). After all that's what get_next_timer_interrupt() does. I'm not sure the above early release of bases locks will bring much in case of contention...
Then a timer_unlock_remote_bases() would restore symmetry.
> +/** > + * timer_lock_remote_bases - lock timer bases of cpu > + * @cpu: Remote CPU > + * > + * Locks the remote timer bases. > + * > + * Returns false if cpu is offline, otherwise true is returned. > + */ > +bool timer_lock_remote_bases(unsigned int cpu) > +{ > + struct timer_base *base_local, *base_global; > + > + /* > + * Pretend that there is no timer pending if the cpu is offline. > + * Possible pending timers will be migrated later to an active cpu. > + */ > + if (cpu_is_offline(cpu)) > + return false;
This value is never checked and the caller assumes the bases are always locked upon calling this (more on this on the big patch).
Thanks.
> + > + base_local = per_cpu_ptr(&timer_bases[BASE_LOCAL], cpu); > + base_global = per_cpu_ptr(&timer_bases[BASE_GLOBAL], cpu); > + > + raw_spin_lock_irq(&base_local->lock); > + raw_spin_lock_nested(&base_global->lock, SINGLE_DEPTH_NESTING); > + > + return true; > +}
| |