diff mbox series

[3/3] arch: um: support virtual time

Message ID 20190503213249.30008-3-johannes@sipsolutions.net
State Superseded
Headers show
Series [1/3] arch: um: fix os_timer_one_shot() | expand

Commit Message

Johannes Berg May 3, 2019, 9:32 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

Sometimes it can be useful to run with virtual time inside the
UML instance, for example for testing. For example, some tests
for the wireless subsystem and userspace are based on hwsim, a
virtual wireless adapter. Some tests can take a long time to
run because they e.g. wait for 120 seconds to elapse for some
regulatory checks. This obviously goes faster if it need not
actually wait that long, but time inside the test environment
just "bumps up" when there's nothing to do.

Add a mode - CONFIG_UML_VIRTUAL_TIME - to support such behavior.

With this enabled, the test mentioned above goes from a runtime
of about 130 seconds (with startup overhead and all) to being
CPU bound and finishing in 16 seconds (on my slow laptop).

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/Kconfig         | 17 +++++++++++++++++
 arch/um/kernel/time.c   |  7 +++++++
 arch/um/os-Linux/time.c | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

Comments

Johannes Berg May 4, 2019, 7:05 p.m. UTC | #1 
On Fri, 2019-05-03 at 21:32 +0000, Johannes Berg wrote:
> 
> @@ -65,12 +75,17 @@ int os_timer_set_interval(void)
> 
>         if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
>                 return -errno;
> +#else
> +       timer_enabled = true;
> +       timer_expiry = virtual_time + UM_NSEC_PER_SEC / UM_HZ;
> +#endif

This is broken in two ways - first of all, I missed the interval
completely, and secondly if you end up with a process that does
something like

 x = gettime();
 while (x == gettime()) { /* nothing */ }

then it gets stuck forever ...

I have a new version almost ready.

johannes
Anton Ivanov May 4, 2019, 8:42 p.m. UTC | #2 
On 04/05/2019 20:05, Johannes Berg wrote:
> On Fri, 2019-05-03 at 21:32 +0000, Johannes Berg wrote:
>>
>> @@ -65,12 +75,17 @@ int os_timer_set_interval(void)
>>
>>          if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
>>                  return -errno;
>> +#else
>> +       timer_enabled = true;
>> +       timer_expiry = virtual_time + UM_NSEC_PER_SEC / UM_HZ;
>> +#endif
> 
> This is broken in two ways - first of all, I missed the interval
> completely, and secondly if you end up with a process that does
> something like
> 
>   x = gettime();
>   while (x == gettime()) { /* nothing */ }
> 
> then it gets stuck forever ...
> 
> I have a new version almost ready.
> 
> johannes
> 
> 
> _______________________________________________
> linux-um mailing list
> linux-um@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um
> 

I had a quick look at them.

1 and 2 are clear and I will most likely ack them once you have a final 
version.

I can understand your use case for patch 3. What I am not clear is where 
should that config option live. It should be somewhere under 
debug/kernel hacking with a sufficient number of big fat warnings so 
people do not enable it accidentally by mistake.

Ideas?
Johannes Berg May 4, 2019, 8:50 p.m. UTC | #3 
On Sat, 2019-05-04 at 21:42 +0100, Anton Ivanov wrote:
> 
> I had a quick look at them.

Thanks.

> 1 and 2 are clear and I will most likely ack them once you have a final 
> version.

I think those two are OK now really. I think I'll put another small bit
into patch 2:

 int os_timer_one_shot(unsigned long ticks)
 {
-       struct itimerspec its;
-       unsigned long long nsec;
-       unsigned long sec;
+       unsigned long long nsec = ticks + 1;
+       struct itimerspec its = {
+               .it_value.tv_sec = nsec / UM_NSEC_PER_SEC,
+               .it_value.tv_nsec = nsec % UM_NSEC_PER_SEC,
 
-       nsec = (ticks + 1);
-       sec = nsec / UM_NSEC_PER_SEC;
-       nsec = nsec % UM_NSEC_PER_SEC;
-
-       its.it_value.tv_sec = nsec / UM_NSEC_PER_SEC;
-       its.it_value.tv_nsec = nsec;
-
-       its.it_interval.tv_sec = 0;
-       its.it_interval.tv_nsec = 0; // we cheat here
+               .it_interval.tv_sec = 0,
+               .it_interval.tv_nsec = 0, // we cheat here
+       };
 

which almost seems like a bugfix too - note how the computed value "sec"
is never used in the original code?


FYI, I also sent a patch to netdev for a fix, just in case you run into
that:
https://patchwork.ozlabs.org/patch/1095009/
(compilation of this is broken right now)

> I can understand your use case for patch 3. What I am not clear is where 
> should that config option live. It should be somewhere under 
> debug/kernel hacking with a sufficient number of big fat warnings so 
> people do not enable it accidentally by mistake.

Yeah, good question. No real objection to moving it there.

There are some quirks with UML still, so I'm trying to figure out if I
can do the same in KVM, but it's kinda cool I can do it at all :-)

johannes
Geert Uytterhoeven May 6, 2019, 7:45 a.m. UTC | #4 
On Sat, May 4, 2019 at 10:43 PM Anton Ivanov
<anton.ivanov@cambridgegreys.com> wrote:
> On 04/05/2019 20:05, Johannes Berg wrote:
> > On Fri, 2019-05-03 at 21:32 +0000, Johannes Berg wrote:
> I can understand your use case for patch 3. What I am not clear is where
> should that config option live. It should be somewhere under
> debug/kernel hacking with a sufficient number of big fat warnings so
> people do not enable it accidentally by mistake.

I think enabling the config option should not also enable the feature, but
just add code that allows to enable the feature by a kernel command line
option, and/or by a special file in debugfs.

Gr{oetje,eeting}s,

                        Geert
Johannes Berg May 6, 2019, 8:19 a.m. UTC | #5 
On Mon, 2019-05-06 at 09:45 +0200, Geert Uytterhoeven wrote:
> On Sat, May 4, 2019 at 10:43 PM Anton Ivanov
> <anton.ivanov@cambridgegreys.com> wrote:
> > On 04/05/2019 20:05, Johannes Berg wrote:
> > > On Fri, 2019-05-03 at 21:32 +0000, Johannes Berg wrote:
> > 
> > I can understand your use case for patch 3. What I am not clear is where
> > should that config option live. It should be somewhere under
> > debug/kernel hacking with a sufficient number of big fat warnings so
> > people do not enable it accidentally by mistake.
> 
> I think enabling the config option should not also enable the feature, but
> just add code that allows to enable the feature by a kernel command line
> option, and/or by a special file in debugfs.

Hmm. Might be doable with a command line argument, I don't think having
the kernel boot in "real time" and then continue running at some point
from "virtual time" when enabled via debugfs makes sense, and I also
don't see how you could really switch back and forth without a lot of
effort in the code.

johannes
diff mbox series

Patch

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index ec9711d068b7..181ffddc03fc 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -180,6 +180,23 @@  config SECCOMP
 
 	  If unsure, say Y.
 
+config UML_VIRTUAL_TIME
+	bool
+	prompt "Use virtual time (e.g. for test execution)"
+	help
+	  Use this option to use virtual time inside the UML instance,
+	  which means that whenever there's nothing to do it just skips
+	  time forward rather than waiting for any real time to elapse.
+
+	  Note that this changes behaviour a bit - used CPU time doesn't
+	  cause the virtual time to increase.
+
+	  When in doubt, say N.
+
+	  In particular, do NOT set this option if you plan on using the
+	  system interactively as that will just cause it to busy-loop
+	  while waiting for you to enter input.
+
 endmenu
 
 source "arch/um/drivers/Kconfig"
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 3e31d7ba84c6..d27e6cf43904 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -134,3 +134,10 @@  void __init time_init(void)
 	timer_set_signal_handler();
 	late_time_init = um_timer_setup;
 }
+
+#ifdef CONFIG_UML_VIRTUAL_TIME
+unsigned long calibrate_delay_is_known(void)
+{
+	return 1;
+}
+#endif
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index 6a60e7506f8d..8d5c0260c627 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -15,8 +15,15 @@ 
 #include <os.h>
 #include <string.h>
 #include <timer-internal.h>
+#include <generated/autoconf.h>
 
+#ifdef CONFIG_UML_VIRTUAL_TIME
+static unsigned long long virtual_time;
+static unsigned long long timer_expiry;
+static bool timer_enabled;
+#else
 static timer_t event_high_res_timer = 0;
+#endif
 
 static inline long long timeval_to_ns(const struct timeval *tv)
 {
@@ -42,16 +49,19 @@  long long os_persistent_clock_emulation(void)
  */
 int os_timer_create(void)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	timer_t *t = &event_high_res_timer;
 
 	if (timer_create(CLOCK_MONOTONIC, NULL, t) == -1)
 		return -1;
+#endif
 
 	return 0;
 }
 
 int os_timer_set_interval(void)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	struct itimerspec its;
 	unsigned long long nsec;
 
@@ -65,12 +75,17 @@  int os_timer_set_interval(void)
 
 	if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
 		return -errno;
+#else
+	timer_enabled = true;
+	timer_expiry = virtual_time + UM_NSEC_PER_SEC / UM_HZ;
+#endif
 
 	return 0;
 }
 
 int os_timer_one_shot(unsigned long ticks)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	struct itimerspec its;
 	unsigned long long nsec;
 	unsigned long sec;
@@ -86,6 +101,11 @@  int os_timer_one_shot(unsigned long ticks)
 	its.it_interval.tv_nsec = 0; // we cheat here
 
 	timer_settime(event_high_res_timer, 0, &its, NULL);
+#else
+	timer_enabled = true;
+	timer_expiry = virtual_time + ticks + 1;
+#endif
+
 	return 0;
 }
 
@@ -94,18 +114,26 @@  int os_timer_one_shot(unsigned long ticks)
  */
 void os_timer_disable(void)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	struct itimerspec its;
 
 	memset(&its, 0, sizeof(struct itimerspec));
 	timer_settime(event_high_res_timer, 0, &its, NULL);
+#else
+	timer_enabled = false;
+#endif
 }
 
 long long os_nsecs(void)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	struct timespec ts;
 
 	clock_gettime(CLOCK_MONOTONIC,&ts);
 	return timespec_to_ns(&ts);
+#else
+	return virtual_time;
+#endif
 }
 
 /**
@@ -114,6 +142,7 @@  long long os_nsecs(void)
  */
 void os_idle_sleep(unsigned long long nsecs)
 {
+#ifndef CONFIG_UML_VIRTUAL_TIME
 	struct timespec ts;
 
 	ts = ((struct timespec) {
@@ -126,4 +155,16 @@  void os_idle_sleep(unsigned long long nsecs)
 	 */
 	if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
 		deliver_alarm();
+#else
+	unsigned long long next = virtual_time + nsecs;
+
+	if (timer_enabled && timer_expiry <= next) {
+		virtual_time = timer_expiry;
+		timer_enabled = false;
+		deliver_alarm();
+		return;
+	}
+
+	virtual_time = next;
+#endif
 }