[PATCH RFC] um: time: fix userspace detection during tick accounting

[PATCH RFC] um: time: fix userspace detection during tick accounting

[Thomas Weißschuh]

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(-)

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,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Re: [PATCH RFC] um: time: fix userspace detection during tick accounting

[Benjamin Berg]

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,