[PATCH 1/3] arch: um: fix os_timer_one_shot()

[PATCH 1/3] arch: um: fix os_timer_one_shot()

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

os_timer_one_shot() gets passed a value "unsigned long delta",
so must not have an "int ticks" as that actually ends up being
-1, and thus triggering a timer over and over again.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/include/shared/os.h | 2 +-
 arch/um/os-Linux/time.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index ebf23012a59b..d579adcb2690 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -252,7 +252,7 @@ extern void os_warn(const char *fmt, ...)
 extern void os_idle_sleep(unsigned long long nsecs);
 extern int os_timer_create(void* timer);
 extern int os_timer_set_interval(void* timer, void* its);
-extern int os_timer_one_shot(int ticks);
+extern int os_timer_one_shot(unsigned long ticks);
 extern long long os_timer_disable(void);
 extern long os_timer_remain(void* timer);
 extern void uml_idle_timer(void);
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index 0e39b9978729..b28cc35da21f 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -113,7 +113,7 @@ long os_timer_remain(void* timer)
 	return its.it_value.tv_nsec;
 }
 
-int os_timer_one_shot(int ticks)
+int os_timer_one_shot(unsigned long ticks)
 {
 	struct itimerspec its;
 	unsigned long long nsec;
-- 
2.17.2


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

[PATCH 2/3] arch: um: timer code cleanup

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

There are some unused functions, and some others that have
unused arguments; clean up the timer code a bit.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/include/shared/os.h |  8 ++-
 arch/um/kernel/time.c       |  4 +-
 arch/um/os-Linux/time.c     | 98 +++++++------------------------------
 3 files changed, 24 insertions(+), 86 deletions(-)

diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index d579adcb2690..449e71edefaa 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -250,15 +250,13 @@ extern void os_warn(const char *fmt, ...)
 
 /* time.c */
 extern void os_idle_sleep(unsigned long long nsecs);
-extern int os_timer_create(void* timer);
-extern int os_timer_set_interval(void* timer, void* its);
+extern int os_timer_create(void);
+extern int os_timer_set_interval(void);
 extern int os_timer_one_shot(unsigned long ticks);
-extern long long os_timer_disable(void);
-extern long os_timer_remain(void* timer);
+extern void os_timer_disable(void);
 extern void uml_idle_timer(void);
 extern long long os_persistent_clock_emulation(void);
 extern long long os_nsecs(void);
-extern long long os_vnsecs(void);
 
 /* skas/mem.c */
 extern long run_syscall_stub(struct mm_id * mm_idp,
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 052de4c8acb2..3e31d7ba84c6 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -37,7 +37,7 @@ static int itimer_shutdown(struct clock_event_device *evt)
 
 static int itimer_set_periodic(struct clock_event_device *evt)
 {
-	os_timer_set_interval(NULL, NULL);
+	os_timer_set_interval();
 	return 0;
 }
 
@@ -107,7 +107,7 @@ static void __init um_timer_setup(void)
 		printk(KERN_ERR "register_timer : request_irq failed - "
 		       "errno = %d\n", -err);
 
-	err = os_timer_create(NULL);
+	err = os_timer_create();
 	if (err != 0) {
 		printk(KERN_ERR "creation of timer failed - errno = %d\n", -err);
 		return;
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index b28cc35da21f..6a60e7506f8d 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -26,11 +26,11 @@ static inline long long timeval_to_ns(const struct timeval *tv)
 
 static inline long long timespec_to_ns(const struct timespec *ts)
 {
-	return ((long long) ts->tv_sec * UM_NSEC_PER_SEC) +
-		ts->tv_nsec;
+	return ((long long) ts->tv_sec * UM_NSEC_PER_SEC) + ts->tv_nsec;
 }
 
-long long os_persistent_clock_emulation (void) {
+long long os_persistent_clock_emulation(void)
+{
 	struct timespec realtime_tp;
 
 	clock_gettime(CLOCK_REALTIME, &realtime_tp);
@@ -40,87 +40,43 @@ long long os_persistent_clock_emulation (void) {
 /**
  * os_timer_create() - create an new posix (interval) timer
  */
-int os_timer_create(void* timer) {
-
-	timer_t* t = timer;
-
-	if(t == NULL) {
-		t = &event_high_res_timer;
-	}
+int os_timer_create(void)
+{
+	timer_t *t = &event_high_res_timer;
 
-	if (timer_create(
-		CLOCK_MONOTONIC,
-		NULL,
-		t) == -1) {
+	if (timer_create(CLOCK_MONOTONIC, NULL, t) == -1)
 		return -1;
-	}
+
 	return 0;
 }
 
-int os_timer_set_interval(void* timer, void* i)
+int os_timer_set_interval(void)
 {
 	struct itimerspec its;
 	unsigned long long nsec;
-	timer_t* t = timer;
-	struct itimerspec* its_in = i;
-
-	if(t == NULL) {
-		t = &event_high_res_timer;
-	}
 
 	nsec = UM_NSEC_PER_SEC / UM_HZ;
 
-	if(its_in != NULL) {
-		its.it_value.tv_sec = its_in->it_value.tv_sec;
-		its.it_value.tv_nsec = its_in->it_value.tv_nsec;
-	} else {
-		its.it_value.tv_sec = 0;
-		its.it_value.tv_nsec = nsec;
-	}
+	its.it_value.tv_sec = 0;
+	its.it_value.tv_nsec = nsec;
 
 	its.it_interval.tv_sec = 0;
 	its.it_interval.tv_nsec = nsec;
 
-	if(timer_settime(*t, 0, &its, NULL) == -1) {
+	if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
 		return -errno;
-	}
 
 	return 0;
 }
 
-/**
- * os_timer_remain() - returns the remaining nano seconds of the given interval
- *                     timer
- * Because this is the remaining time of an interval timer, which correspondends
- * to HZ, this value can never be bigger than one second. Just
- * the nanosecond part of the timer is returned.
- * The returned time is relative to the start time of the interval timer.
- * Return an negative value in an error case.
- */
-long os_timer_remain(void* timer)
-{
-	struct itimerspec its;
-	timer_t* t = timer;
-
-	if(t == NULL) {
-		t = &event_high_res_timer;
-	}
-
-	if(timer_gettime(t, &its) == -1) {
-		return -errno;
-	}
-
-	return its.it_value.tv_nsec;
-}
-
 int os_timer_one_shot(unsigned long ticks)
 {
 	struct itimerspec its;
 	unsigned long long nsec;
 	unsigned long sec;
 
-    nsec = (ticks + 1);
-    sec = 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;
@@ -135,24 +91,13 @@ int os_timer_one_shot(unsigned long ticks)
 
 /**
  * os_timer_disable() - disable the posix (interval) timer
- * Returns the remaining interval timer time in nanoseconds
  */
-long long os_timer_disable(void)
+void os_timer_disable(void)
 {
 	struct itimerspec its;
 
 	memset(&its, 0, sizeof(struct itimerspec));
-	timer_settime(event_high_res_timer, 0, &its, &its);
-
-	return its.it_value.tv_sec * UM_NSEC_PER_SEC + its.it_value.tv_nsec;
-}
-
-long long os_vnsecs(void)
-{
-	struct timespec ts;
-
-	clock_gettime(CLOCK_PROCESS_CPUTIME_ID,&ts);
-	return timespec_to_ns(&ts);
+	timer_settime(event_high_res_timer, 0, &its, NULL);
 }
 
 long long os_nsecs(void)
@@ -171,19 +116,14 @@ void os_idle_sleep(unsigned long long nsecs)
 {
 	struct timespec ts;
 
-	if (nsecs <= 0) {
-		return;
-	}
-
 	ts = ((struct timespec) {
-			.tv_sec  = nsecs / UM_NSEC_PER_SEC,
-			.tv_nsec = nsecs % UM_NSEC_PER_SEC
+		.tv_sec  = nsecs / UM_NSEC_PER_SEC,
+		.tv_nsec = nsecs % UM_NSEC_PER_SEC
 	});
 
 	/*
 	 * Relay the signal if clock_nanosleep is interrupted.
 	 */
-	if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL)) {
+	if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
 		deliver_alarm();
-	}
 }
-- 
2.17.2


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

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

[Johannes Berg]

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

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
 }
-- 
2.17.2


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

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

[Johannes Berg]

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

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

[Anton Ivanov]

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?

-- 
Anton R. Ivanov

Cambridge Greys Limited, England and Wales company No 10273661
http://www.cambridgegreys.com/

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

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

[Johannes Berg]

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


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

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

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Johannes Berg]

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