Messages in this thread Patch in this message | | | From | Vincent Whitchurch <> | Date | Fri, 20 Oct 2023 16:47:39 +0200 | Subject | [PATCH] um: time-travel: fix time going backwards |
| |
In basic time travel mode, I sometimes see "time goes backwards" panics like the one below:
Kernel panic: time-travel: time goes backwards 161689340000492 -> 161689339869814 Call Trace: panic+0x1a1/0x3d7 time_travel_update_time.cold+0xe9/0x133 timer_read+0xc1/0x100 ktime_get+0x10c/0x200 copy_process+0x1899/0x2230 kernel_clone+0x57/0x7a0 kernel_thread+0x4a/0x50 kthreadd+0x116/0x190
The problem is a race between time_travel_handle_real_alarm() and timer_read(). time_travel_handle_real_alarm() changes the time after time_read() reads the current time but before time_travel_update_time() has had a chance to add the end event.
Fix this by doing the time read and event add atomically with respect to time_travel_handle_real_alarm().
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- arch/um/kernel/time.c | 48 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c index fddd1dec27e6..38a94dd41b8f 100644 --- a/arch/um/kernel/time.c +++ b/arch/um/kernel/time.c @@ -392,17 +392,11 @@ bool time_travel_del_event(struct time_travel_event *e) return true; } -static void time_travel_update_time(unsigned long long next, bool idle) +static void time_travel_update_to_event(struct time_travel_event *ne, bool idle) { - struct time_travel_event ne = { - .onstack = true, - }; struct time_travel_event *e; bool finished = idle; - /* add it without a handler - we deal with that specifically below */ - __time_travel_add_event(&ne, next); - do { e = time_travel_first_event(); @@ -414,7 +408,7 @@ static void time_travel_update_time(unsigned long long next, bool idle) BUG_ON(!time_travel_del_event(e)); BUG_ON(time_travel_time != e->time); - if (e == &ne) { + if (e == ne) { finished = true; } else { if (e->onstack) @@ -427,14 +421,38 @@ static void time_travel_update_time(unsigned long long next, bool idle) e = time_travel_first_event(); if (e) time_travel_ext_update_request(e->time); - } while (ne.pending && !finished); + } while (ne->pending && !finished); + + time_travel_del_event(ne); +} - time_travel_del_event(&ne); +static void time_travel_update_time(unsigned long long next, bool idle) +{ + struct time_travel_event ne = { + .onstack = true, + }; + + __time_travel_add_event(&ne, next); + time_travel_update_to_event(&ne, idle); +} + +static void time_travel_forward_time(unsigned long nsec, bool idle) +{ + struct time_travel_event ne = { + .onstack = true, + }; + unsigned long flags; + + local_irq_save(flags); + __time_travel_add_event(&ne, time_travel_time + nsec); + local_irq_restore(flags); + + time_travel_update_to_event(&ne, idle); } void time_travel_ndelay(unsigned long nsec) { - time_travel_update_time(time_travel_time + nsec, false); + time_travel_forward_time(nsec, false); } EXPORT_SYMBOL(time_travel_ndelay); @@ -572,6 +590,10 @@ static inline void time_travel_update_time(unsigned long long ns, bool retearly) { } +static void time_travel_forward_time(unsigned long nsec, bool idle) +{ +} + static inline void time_travel_handle_real_alarm(void) { } @@ -720,9 +742,7 @@ static u64 timer_read(struct clocksource *cs) */ if (!irqs_disabled() && !in_interrupt() && !in_softirq() && !time_travel_ext_waiting) - time_travel_update_time(time_travel_time + - TIMER_MULTIPLIER, - false); + time_travel_forward_time(TIMER_MULTIPLIER, false); return time_travel_time / TIMER_MULTIPLIER; } --- base-commit: 58720809f52779dc0f08e53e54b014209d13eebb change-id: 20231020-uml-time-backwards-552c81aedd23 Best regards, -- Vincent Whitchurch <vincent.whitchurch@axis.com>
| |