From: Vincent Whitchurch <vincent.whitchurch@axis.com>
To: Richard Weinberger <richard@nod.at>,
Anton Ivanov <anton.ivanov@cambridgegreys.com>,
Johannes Berg <johannes@sipsolutions.net>
Cc: <linux-um@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
<kernel@axis.com>,
Vincent Whitchurch <vincent.whitchurch@axis.com>
Subject: [PATCH] um: time-travel: fix time going backwards
Date: Fri, 20 Oct 2023 16:47:39 +0200 [thread overview]
Message-ID: <20231020-uml-time-backwards-v1-1-90b776fc6dfd@axis.com> (raw)
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>
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
WARNING: multiple messages have this Message-ID (diff)
From: Vincent Whitchurch <vincent.whitchurch@axis.com>
To: Richard Weinberger <richard@nod.at>,
Anton Ivanov <anton.ivanov@cambridgegreys.com>,
Johannes Berg <johannes@sipsolutions.net>
Cc: <linux-um@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
<kernel@axis.com>,
Vincent Whitchurch <vincent.whitchurch@axis.com>
Subject: [PATCH] um: time-travel: fix time going backwards
Date: Fri, 20 Oct 2023 16:47:39 +0200 [thread overview]
Message-ID: <20231020-uml-time-backwards-v1-1-90b776fc6dfd@axis.com> (raw)
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>
next reply other threads:[~2023-10-20 14:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-20 14:47 Vincent Whitchurch [this message]
2023-10-20 14:47 ` [PATCH] um: time-travel: fix time going backwards Vincent Whitchurch
2023-10-23 7:08 ` Vincent Whitchurch
2023-10-23 7:08 ` Vincent Whitchurch
2023-10-23 7:33 ` Johannes Berg
2023-10-23 7:33 ` Johannes Berg
2023-10-25 11:55 ` Vincent Whitchurch
2023-10-25 11:55 ` Vincent Whitchurch
2023-10-25 19:51 ` Johannes Berg
2023-10-25 19:51 ` Johannes Berg
2023-10-25 20:02 ` Johannes Berg
2023-10-25 20:02 ` Johannes Berg
2023-10-25 20:41 ` Johannes Berg
2023-10-25 20:41 ` Johannes Berg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231020-uml-time-backwards-v1-1-90b776fc6dfd@axis.com \
--to=vincent.whitchurch@axis.com \
--cc=anton.ivanov@cambridgegreys.com \
--cc=johannes@sipsolutions.net \
--cc=kernel@axis.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=richard@nod.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.