| Message ID | 20210524221818.15850-3-will@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tick/broadcast: Allow per-cpu timers to be used instead of broadcast | expand |
Hi will: I had backport you tick/broadcast: Prefer per-cpu relatives patches, but i did not get the true result, the Wakeup Devices are all null, why? my machine is armv8-a arm64 #cat current_clocksource arch_sys_counter my local clock event is Tick Device: mode: 1 Per CPU device: 95 Clock Event Device: arch_sys_timer max_delta_ns: 21474836451 min_delta_ns: 1000 mult: 429496730 shift: 32 mode: 3 next_event: 14951080000000 nsecs set_next_event: arch_timer_set_next_event_phys shutdown: arch_timer_shutdown_phys oneshot stopped: arch_timer_shutdown_phys event_handler: hrtimer_interrupt retries: 70 Wakeup Device: <NULL> cat /proc/timer_list | grep "Wakeup Device:" Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL> Wakeup Device: <NULL>
On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: > I had backport you tick/broadcast: Prefer per-cpu relatives patches, > > but i did not get the true result, the Wakeup Devices are all null, why? Probably because you don't have any suitable per-cpu timers to act as a wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU and CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_C3STOP? Will
在 2021/5/27 下午4:22, Will Deacon 写道: > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: >> I had backport you tick/broadcast: Prefer per-cpu relatives patches, >> >> but i did not get the true result, the Wakeup Devices are all null, why? > Probably because you don't have any suitable per-cpu timers to act as a > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU Yes, you are right, but i want to know why the timer do not support CLOCK_EVT_FEAT_PERCPU. > and CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_C3STOP? > > Will
On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote: > > 在 2021/5/27 下午4:22, Will Deacon 写道: > > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: > > > I had backport you tick/broadcast: Prefer per-cpu relatives patches, > > > > > > but i did not get the true result, the Wakeup Devices are all null, why? > > Probably because you don't have any suitable per-cpu timers to act as a > > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU > > Yes, you are right, but i want to know why the timer do not support > CLOCK_EVT_FEAT_PERCPU. I suspect there may be drivers with the feature missing simply because it wasn't really used for much until now (I think it just prevented use for broadcast). However, before adding that to a timer, you do need to make sure that it really is banked per-cpu (or at least handles racing accesses) as well as having the per-cpu irq. Will
On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote: > > 在 2021/5/27 下午4:22, Will Deacon 写道: > > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: > > > I had backport you tick/broadcast: Prefer per-cpu relatives patches, > > > > > > but i did not get the true result, the Wakeup Devices are all null, why? > > Probably because you don't have any suitable per-cpu timers to act as a > > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU > > Yes, you are right, but i want to know why the timer do not support > CLOCK_EVT_FEAT_PERCPU. I defer to Thomas on this one. The approach I have taken here (of printing <NULL>) follows what is done elsewhere for the timer readout and probably helps with parsing this stuff. Will
On Thu, May 27 2021 at 12:56, Will Deacon wrote: > On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote: >> >> 在 2021/5/27 下午4:22, Will Deacon 写道: >> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: >> > > I had backport you tick/broadcast: Prefer per-cpu relatives patches, >> > > >> > > but i did not get the true result, the Wakeup Devices are all null, why? >> > Probably because you don't have any suitable per-cpu timers to act as a >> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU >> >> Yes, you are right, but i want to know why the timer do not support >> CLOCK_EVT_FEAT_PERCPU. > > I defer to Thomas on this one. How should I know what kind of timers this hardware has? Thanks, tglx
Hi Thomas, On Mon, May 31, 2021 at 04:29:20PM +0200, Thomas Gleixner wrote: > On Thu, May 27 2021 at 12:56, Will Deacon wrote: > > > On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote: > >> > >> 在 2021/5/27 下午4:22, Will Deacon 写道: > >> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: > >> > > I had backport you tick/broadcast: Prefer per-cpu relatives patches, > >> > > > >> > > but i did not get the true result, the Wakeup Devices are all null, why? > >> > Probably because you don't have any suitable per-cpu timers to act as a > >> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU > >> > >> Yes, you are right, but i want to know why the timer do not support > >> CLOCK_EVT_FEAT_PERCPU. > > > > I defer to Thomas on this one. > > How should I know what kind of timers this hardware has? Duh, sorry, I replied to the wrong question. I meant to defer the decision about whether to print "<NULL>" if the wakeup timer is absent, or whether to omit the line entirely. I went with the former in the patches you queued as it's both consistent with the rest of the code and probably (?) easier to parse. Will
On Tue, Jun 01 2021 at 13:13, Will Deacon wrote: > On Mon, May 31, 2021 at 04:29:20PM +0200, Thomas Gleixner wrote: >> On Thu, May 27 2021 at 12:56, Will Deacon wrote: >> >> > On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote: >> >> >> >> 在 2021/5/27 下午4:22, Will Deacon 写道: >> >> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote: >> >> > > I had backport you tick/broadcast: Prefer per-cpu relatives patches, >> >> > > >> >> > > but i did not get the true result, the Wakeup Devices are all null, why? >> >> > Probably because you don't have any suitable per-cpu timers to act as a >> >> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU >> >> >> >> Yes, you are right, but i want to know why the timer do not support >> >> CLOCK_EVT_FEAT_PERCPU. >> > >> > I defer to Thomas on this one. >> >> How should I know what kind of timers this hardware has? > > Duh, sorry, I replied to the wrong question. I meant to defer the decision > about whether to print "<NULL>" if the wakeup timer is absent, or whether to > omit the line entirely. > > I went with the former in the patches you queued as it's both consistent > with the rest of the code and probably (?) easier to parse. That makes more sense. I just kept it as is. The <NULL> is fine. Thanks, tglx
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c index fb794ff4855e..f3f2f4ba4321 100644 --- a/kernel/time/tick-broadcast.c +++ b/kernel/time/tick-broadcast.c @@ -717,24 +717,16 @@ static void broadcast_shutdown_local(struct clock_event_device *bc, clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN); } -int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) +static int ___tick_broadcast_oneshot_control(enum tick_broadcast_state state, + struct tick_device *td, + int cpu) { - struct clock_event_device *bc, *dev; - int cpu, ret = 0; + struct clock_event_device *bc, *dev = td->evtdev; + int ret = 0; ktime_t now; - /* - * If there is no broadcast device, tell the caller not to go - * into deep idle. - */ - if (!tick_broadcast_device.evtdev) - return -EBUSY; - - dev = this_cpu_ptr(&tick_cpu_device)->evtdev; - raw_spin_lock(&tick_broadcast_lock); bc = tick_broadcast_device.evtdev; - cpu = smp_processor_id(); if (state == TICK_BROADCAST_ENTER) { /* @@ -863,6 +855,21 @@ int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) return ret; } +int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) +{ + struct tick_device *td = this_cpu_ptr(&tick_cpu_device); + int cpu = smp_processor_id(); + + if (tick_broadcast_device.evtdev) + return ___tick_broadcast_oneshot_control(state, td, cpu); + + /* + * If there is no broadcast device, tell the caller not + * to go into deep idle. + */ + return -EBUSY; +} + /* * Reset the one shot broadcast for a cpu *
In preparation for adding support for per-cpu wakeup timers, split _tick_broadcast_oneshot_control() into a helper function which deals only with the broadcast timer management across idle transitions. Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Marc Zyngier <maz@kernel.org> Cc: Lorenzo Colitti <lorenzo@google.com> Cc: John Stultz <john.stultz@linaro.org> Cc: Stephen Boyd <sboyd@kernel.org> Signed-off-by: Will Deacon <will@kernel.org> --- kernel/time/tick-broadcast.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)