Messages in this thread Patch in this message | | | Subject | Re: [patch V2 02/10] hrtimer: Consolidate reprogramming code | From | Marek Szyprowski <> | Date | Thu, 12 Aug 2021 15:09:44 +0200 |
| |
Hi,
On 13.07.2021 15:39, Thomas Gleixner wrote: > From: Peter Zijlstra <peterz@infradead.org> > > This code is mostly duplicated. The redudant store in the force reprogram > case does no harm and the in hrtimer interrupt condition cannot be true for > the force reprogram invocations. > > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > kernel/time/hrtimer.c | 72 ++++++++++++++++++++------------------------------ > 1 file changed, 29 insertions(+), 43 deletions(-) > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
This patch landed in today's linux-next (next-20210812) as commit b14bca97c9f5 ("hrtimer: Consolidate reprogramming code"). It breaks booting of many of my test machines: ARM 32bit Exynos based boards, ARM 64bit QEmu virt machine or ARM64 Qualcomm DragonBoard410c board.
I've managed to catch the following log on QEmu's virt ARM64 machine:
rcu: INFO: rcu_preempt detected stalls on CPUs/tasks: rcu: 0-...!: (0 ticks this GP) idle=330/0/0x0 softirq=42/42 fqs=0 (false positive?) (detected by 1, t=6502 jiffies, g=-1091, q=115)
============================================ WARNING: possible recursive locking detected 5.14.0-rc5+ #10668 Not tainted -------------------------------------------- swapper/1/0 is trying to acquire lock: ffffbb9c1e4ca1d8 (rcu_node_0){-.-.}-{2:2}, at: rcu_dump_cpu_stacks+0x68/0x1c4
but task is already holding lock: ffffbb9c1e4ca1d8 (rcu_node_0){-.-.}-{2:2}, at: rcu_sched_clock_irq+0x83c/0x1778
other info that might help us debug this: Possible unsafe locking scenario:
CPU0 ---- lock(rcu_node_0); lock(rcu_node_0);
*** DEADLOCK ***
May be due to missing lock nesting notation
1 lock held by swapper/1/0: #0: ffffbb9c1e4ca1d8 (rcu_node_0){-.-.}-{2:2}, at: rcu_sched_clock_irq+0x83c/0x1778
stack backtrace: CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.14.0-rc5+ #10668 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x1d0 show_stack+0x14/0x20 dump_stack_lvl+0x88/0xb0 dump_stack+0x14/0x2c __lock_acquire+0x17a4/0x1840 lock_acquire+0x130/0x3e8 _raw_spin_lock_irqsave+0x78/0x148 rcu_dump_cpu_stacks+0x68/0x1c4 rcu_sched_clock_irq+0x11e8/0x1778 update_process_times+0x88/0xd0 tick_sched_handle.isra.19+0x30/0x50 tick_sched_timer+0x48/0x98 __hrtimer_run_queues+0x380/0x5b0 hrtimer_interrupt+0xe4/0x240 arch_timer_handler_virt+0x30/0x40 handle_percpu_devid_irq+0xc0/0x3d0 handle_domain_irq+0x58/0x88 gic_handle_irq+0xa8/0xc8 call_on_irq_stack+0x28/0x38 do_interrupt_handler+0x54/0x60 el1_interrupt+0x2c/0x108 el1h_64_irq_handler+0x14/0x20 el1h_64_irq+0x74/0x78 arch_cpu_idle+0x14/0x20 default_idle_call+0x88/0x390 do_idle+0x200/0x290 cpu_startup_entry+0x20/0x80 secondary_start_kernel+0x1c0/0x1f0 __secondary_switched+0x7c/0x80
I hope it helps fixing the issue.
> --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -652,21 +652,24 @@ static inline int hrtimer_hres_active(vo > return __hrtimer_hres_active(this_cpu_ptr(&hrtimer_bases)); > } > > -/* > - * Reprogram the event source with checking both queues for the > - * next event > - * Called with interrupts disabled and base->lock held > - */ > static void > -hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal) > +__hrtimer_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal, > + struct hrtimer *next_timer, ktime_t expires_next) > { > - ktime_t expires_next; > + /* > + * If the hrtimer interrupt is running, then it will reevaluate the > + * clock bases and reprogram the clock event device. > + */ > + if (cpu_base->in_hrtirq) > + return; > > - expires_next = hrtimer_update_next_event(cpu_base); > + if (expires_next > cpu_base->expires_next) > + return; > > if (skip_equal && expires_next == cpu_base->expires_next) > return; > > + cpu_base->next_timer = next_timer; > cpu_base->expires_next = expires_next; > > /* > @@ -689,7 +692,23 @@ hrtimer_force_reprogram(struct hrtimer_c > if (!__hrtimer_hres_active(cpu_base) || cpu_base->hang_detected) > return; > > - tick_program_event(cpu_base->expires_next, 1); > + tick_program_event(expires_next, 1); > +} > + > +/* > + * Reprogram the event source with checking both queues for the > + * next event > + * Called with interrupts disabled and base->lock held > + */ > +static void > +hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal) > +{ > + ktime_t expires_next; > + > + expires_next = hrtimer_update_next_event(cpu_base); > + > + __hrtimer_reprogram(cpu_base, skip_equal, cpu_base->next_timer, > + expires_next); > } > > /* High resolution timer related functions */ > @@ -835,40 +854,7 @@ static void hrtimer_reprogram(struct hrt > if (base->cpu_base != cpu_base) > return; > > - /* > - * If the hrtimer interrupt is running, then it will > - * reevaluate the clock bases and reprogram the clock event > - * device. The callbacks are always executed in hard interrupt > - * context so we don't need an extra check for a running > - * callback. > - */ > - if (cpu_base->in_hrtirq) > - return; > - > - if (expires >= cpu_base->expires_next) > - return; > - > - /* Update the pointer to the next expiring timer */ > - cpu_base->next_timer = timer; > - cpu_base->expires_next = expires; > - > - /* > - * If hres is not active, hardware does not have to be > - * programmed yet. > - * > - * If a hang was detected in the last timer interrupt then we > - * do not schedule a timer which is earlier than the expiry > - * which we enforced in the hang detection. We want the system > - * to make progress. > - */ > - if (!__hrtimer_hres_active(cpu_base) || cpu_base->hang_detected) > - return; > - > - /* > - * Program the timer hardware. We enforce the expiry for > - * events which are already in the past. > - */ > - tick_program_event(expires, 1); > + __hrtimer_reprogram(cpu_base, true, timer, expires); > } > > /* > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
| |