| Message ID | 20250617-uml-tick-timer-v1-1-01aab312f56b@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] um: time: fix userspace detection during tick accounting | expand |
Hi, On Tue, 2025-06-17 at 15:52 +0200, Thomas Weißschuh wrote: > The cpu usage timekeeping triggered by the tick credits passed time to > either the kernel or the currently running userspace process. > On UML, tick interrupts firing while userspace is running are not marked > correctly so this bookkeeping is broken and always credits the idle task. > > The rootcause is the following callchain always passing user_tick=false > to account_process_tick(): > > um_timer() > -> handle_irq_event() > -> tick_handle_periodic() > -> tick_periodic() > -> update_process_times(user_tick=user_mode(get_irq_regs())) > -> account_process_tick(task, user_tick=false) > > As a result CPUCLOCK_VIRT does not advance, > breaking for example signal(SIGVTALRM). > > The issue can be reproduced easily with the selftest > tools/testing/selftests/timers/posix_timers.c, > which hangs in the ITIMER_VIRTUAL/SIGVTALRM testcase. > > Fix up the IRQ regs by correctly setting is_user in the IRQ registers. So, in principle, this looks like the right thing. What I am not sure about is accounting if the userspace task is currently executing a syscall inside the kernel thread. The current code will incorrectly interrupt the userspace process in that case and you are now adding CPU time accounting on top of the same logic that is already not quite right. Said differently, need to think about it a bit more. I suspect we might need to set/clear a flag in the userspace function for this. Benjamin > > Fixes: 2eb5f31bc4ea ("um: Switch clocksource to hrtimers") > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > --- > I'm not familiar with UML, so this is probably not the right fix. > Feel free to treat it as a bugreport instead. > --- > arch/um/kernel/time.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c > index ae0fa2173778f43273fd0550f77fc99bbb1c3e3a..a6c17302403aeb3170a1104117c4e713e3afdfe0 100644 > --- a/arch/um/kernel/time.c > +++ b/arch/um/kernel/time.c > @@ -856,6 +856,7 @@ static struct clock_event_device timer_clockevent = { > > static irqreturn_t um_timer(int irq, void *dev) > { > + struct pt_regs *regs; > /* > * Interrupt the (possibly) running userspace process, technically this > * should only happen if userspace is currently executing. > @@ -864,9 +865,13 @@ static irqreturn_t um_timer(int irq, void *dev) > */ > if (time_travel_mode != TT_MODE_INFCPU && > time_travel_mode != TT_MODE_EXTERNAL && > - get_current()->mm) > + get_current()->mm) { > os_alarm_process(get_current()->mm->context.id.pid); > > + regs = get_irq_regs(); > + regs->regs.is_user = true; > + } > + > (*timer_clockevent.event_handler)(&timer_clockevent); > > return IRQ_HANDLED; > > --- > base-commit: 9afe652958c3ee88f24df1e4a97f298afce89407 > change-id: 20250617-uml-tick-timer-82ea89495cc6 > > Best regards,
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c index ae0fa2173778f43273fd0550f77fc99bbb1c3e3a..a6c17302403aeb3170a1104117c4e713e3afdfe0 100644 --- a/arch/um/kernel/time.c +++ b/arch/um/kernel/time.c @@ -856,6 +856,7 @@ static struct clock_event_device timer_clockevent = { static irqreturn_t um_timer(int irq, void *dev) { + struct pt_regs *regs; /* * Interrupt the (possibly) running userspace process, technically this * should only happen if userspace is currently executing. @@ -864,9 +865,13 @@ static irqreturn_t um_timer(int irq, void *dev) */ if (time_travel_mode != TT_MODE_INFCPU && time_travel_mode != TT_MODE_EXTERNAL && - get_current()->mm) + get_current()->mm) { os_alarm_process(get_current()->mm->context.id.pid); + regs = get_irq_regs(); + regs->regs.is_user = true; + } + (*timer_clockevent.event_handler)(&timer_clockevent); return IRQ_HANDLED;
The cpu usage timekeeping triggered by the tick credits passed time to either the kernel or the currently running userspace process. On UML, tick interrupts firing while userspace is running are not marked correctly so this bookkeeping is broken and always credits the idle task. The rootcause is the following callchain always passing user_tick=false to account_process_tick(): um_timer() -> handle_irq_event() -> tick_handle_periodic() -> tick_periodic() -> update_process_times(user_tick=user_mode(get_irq_regs())) -> account_process_tick(task, user_tick=false) As a result CPUCLOCK_VIRT does not advance, breaking for example signal(SIGVTALRM). The issue can be reproduced easily with the selftest tools/testing/selftests/timers/posix_timers.c, which hangs in the ITIMER_VIRTUAL/SIGVTALRM testcase. Fix up the IRQ regs by correctly setting is_user in the IRQ registers. Fixes: 2eb5f31bc4ea ("um: Switch clocksource to hrtimers") Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> --- I'm not familiar with UML, so this is probably not the right fix. Feel free to treat it as a bugreport instead. --- arch/um/kernel/time.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) --- base-commit: 9afe652958c3ee88f24df1e4a97f298afce89407 change-id: 20250617-uml-tick-timer-82ea89495cc6 Best regards,