All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] arch: um: fixes and virtual time support
@ 2019-05-06 12:39 Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 1/5] arch: um: Fix IRQ controller regression on console read Johannes Berg
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j

Here's a respin, with some comments addressed.

I'm also including a patch from Jouni to fix an IRQ controller
regression, since we've been working together a bit on using
this to run hwsim (CONFIG_MAC80211_HWSIM) based tests in UML
virtual time for speedups.

johannes



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/5] arch: um: Fix IRQ controller regression on console read
  2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
@ 2019-05-06 12:39 ` Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 2/5] arch: um: use cpu_possible_mask to avoid warning Johannes Berg
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j

From: Jouni Malinen <j@w1.fi>

The conversion of UML to use epoll based IRQ controller claimed that
clone_one_chan() can safely call um_free_irq() while starting to ignore
the delay_free_irq parameter that explicitly noted that the IRQ cannot
be freed because this is being called from chan_interrupt(). This
resulted in free_irq() getting called in interrupt context ("Trying to
free IRQ 6 from IRQ context!").

Fix this by restoring previously used delay_free_irq processing.

Fixes: ff6a17989c08 ("Epoll based IRQ controller")
Signed-off-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/drivers/chan_kern.c | 52 +++++++++++++++++++++++++++++++------
 arch/um/kernel/irq.c        |  4 +++
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/arch/um/drivers/chan_kern.c b/arch/um/drivers/chan_kern.c
index a4e64edb8f38..749d2bf59599 100644
--- a/arch/um/drivers/chan_kern.c
+++ b/arch/um/drivers/chan_kern.c
@@ -171,19 +171,55 @@ int enable_chan(struct line *line)
 	return err;
 }
 
+/* Items are added in IRQ context, when free_irq can't be called, and
+ * removed in process context, when it can.
+ * This handles interrupt sources which disappear, and which need to
+ * be permanently disabled.  This is discovered in IRQ context, but
+ * the freeing of the IRQ must be done later.
+ */
+static DEFINE_SPINLOCK(irqs_to_free_lock);
+static LIST_HEAD(irqs_to_free);
+
+void free_irqs(void)
+{
+	struct chan *chan;
+	LIST_HEAD(list);
+	struct list_head *ele;
+	unsigned long flags;
+
+	spin_lock_irqsave(&irqs_to_free_lock, flags);
+	list_splice_init(&irqs_to_free, &list);
+	spin_unlock_irqrestore(&irqs_to_free_lock, flags);
+
+	list_for_each(ele, &list) {
+		chan = list_entry(ele, struct chan, free_list);
+
+		if (chan->input && chan->enabled)
+			um_free_irq(chan->line->driver->read_irq, chan);
+		if (chan->output && chan->enabled)
+			um_free_irq(chan->line->driver->write_irq, chan);
+		chan->enabled = 0;
+	}
+}
+
 static void close_one_chan(struct chan *chan, int delay_free_irq)
 {
+	unsigned long flags;
+
 	if (!chan->opened)
 		return;
 
-    /* we can safely call free now - it will be marked
-     *  as free and freed once the IRQ stopped processing
-     */
-	if (chan->input && chan->enabled)
-		um_free_irq(chan->line->driver->read_irq, chan);
-	if (chan->output && chan->enabled)
-		um_free_irq(chan->line->driver->write_irq, chan);
-	chan->enabled = 0;
+	if (delay_free_irq) {
+		spin_lock_irqsave(&irqs_to_free_lock, flags);
+		list_add(&chan->free_list, &irqs_to_free);
+		spin_unlock_irqrestore(&irqs_to_free_lock, flags);
+	} else {
+		if (chan->input && chan->enabled)
+			um_free_irq(chan->line->driver->read_irq, chan);
+		if (chan->output && chan->enabled)
+			um_free_irq(chan->line->driver->write_irq, chan);
+		chan->enabled = 0;
+	}
 	if (chan->ops->close != NULL)
 		(*chan->ops->close)(chan->fd, chan->data);
 
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index f4874b7ec503..52444be9f162 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -21,6 +21,8 @@
 #include <irq_user.h>
 
 
+extern void free_irqs(void);
+
 /* When epoll triggers we do not know why it did so
  * we can also have different IRQs for read and write.
  * This is why we keep a small irq_fd array for each fd -
@@ -100,6 +102,8 @@ void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
 			}
 		}
 	}
+
+	free_irqs();
 }
 
 static int assign_epoll_events_to_irq(struct irq_entry *irq_entry)
-- 
2.17.2


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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/5] arch: um: use cpu_possible_mask to avoid warning
  2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 1/5] arch: um: Fix IRQ controller regression on console read Johannes Berg
@ 2019-05-06 12:39 ` Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 3/5] arch: um: fix os_timer_one_shot() Johannes Berg
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j, Johannes Berg

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

Using cpu_all_mask in a struct clock_event_device leads to
a warning from clockevents_register_device(), fix this.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/kernel/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 052de4c8acb2..0c572a48158e 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -56,7 +56,7 @@ static int itimer_one_shot(struct clock_event_device *evt)
 static struct clock_event_device timer_clockevent = {
 	.name			= "posix-timer",
 	.rating			= 250,
-	.cpumask		= cpu_all_mask,
+	.cpumask		= cpu_possible_mask,
 	.features		= CLOCK_EVT_FEAT_PERIODIC |
 				  CLOCK_EVT_FEAT_ONESHOT,
 	.set_state_shutdown	= itimer_shutdown,
-- 
2.17.2


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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/5] arch: um: fix os_timer_one_shot()
  2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 1/5] arch: um: Fix IRQ controller regression on console read Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 2/5] arch: um: use cpu_possible_mask to avoid warning Johannes Berg
@ 2019-05-06 12:39 ` Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 4/5] arch: um: timer code cleanup Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 5/5] arch: um: support virtual time Johannes Berg
  4 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/5] arch: um: timer code cleanup
  2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
                   ` (2 preceding siblings ...)
  2019-05-06 12:39 ` [PATCH v2 3/5] arch: um: fix os_timer_one_shot() Johannes Berg
@ 2019-05-06 12:39 ` Johannes Berg
  2019-05-06 12:39 ` [PATCH v2 5/5] arch: um: support virtual time Johannes Berg
  4 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j, 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     | 119 ++++++++----------------------------
 3 files changed, 31 insertions(+), 100 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 0c572a48158e..3898119f773e 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..ea720149f5b8 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,94 +40,45 @@ 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;
+	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
+	};
 
 	timer_settime(event_high_res_timer, 0, &its, NULL);
 	return 0;
@@ -135,24 +86,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)
@@ -169,21 +109,14 @@ long long os_nsecs(void)
  */
 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
-	});
+	struct timespec ts = {
+		.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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/5] arch: um: support virtual time
  2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
                   ` (3 preceding siblings ...)
  2019-05-06 12:39 ` [PATCH v2 4/5] arch: um: timer code cleanup Johannes Berg
@ 2019-05-06 12:39 ` Johannes Berg
  2019-05-26 21:55   ` Richard Weinberger
  4 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2019-05-06 12:39 UTC (permalink / raw)
  To: linux-um; +Cc: j, 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_SUPPORT - to support such
behavior; it needs to be enabled with the "virtual-time" option
passed to the UML invocation.

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/include/shared/os.h | 14 ++++++++
 arch/um/kernel/time.c       | 13 +++++++
 arch/um/os-Linux/time.c     | 68 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index ec9711d068b7..71ff7ef3aa0c 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -180,6 +180,23 @@ config SECCOMP
 
 	  If unsure, say Y.
 
+config UML_VIRTUAL_TIME_SUPPORT
+	bool
+	prompt "Support virtual time (e.g. for test execution)"
+	help
+	  Enable this option to support 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 may not always
+	  cause the virtual time to increase unless enough CPU was consumed to
+	  advance the tick (HZ).
+
+	  Note that to enable the virtual time, you also need to pass
+	  "virtual-time" on the command-line.
+
+	  It is safe to say Y, but you probably don't need this, so say N.
+
 endmenu
 
 source "arch/um/drivers/Kconfig"
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 449e71edefaa..a891a5665704 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -257,6 +257,20 @@ 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);
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+extern unsigned long long virtual_time;
+extern unsigned long long timer_expiry;
+int os_setup_virtual_time(char *str);
+static inline void os_set_virtual_time_to_timer(void)
+{
+	/* ignored if virtual time isn't enabled */
+	virtual_time = timer_expiry;
+}
+#else
+static inline void os_set_virtual_time_to_timer(void)
+{
+}
+#endif
 
 /* 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 3898119f773e..0ceb7c540d60 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -19,11 +19,14 @@
 #include <kern_util.h>
 #include <os.h>
 #include <timer-internal.h>
+#include <shared/init.h>
 
 void timer_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
 {
 	unsigned long flags;
 
+	os_set_virtual_time_to_timer();
+
 	local_irq_save(flags);
 	do_IRQ(TIMER_IRQ, regs);
 	local_irq_restore(flags);
@@ -134,3 +137,13 @@ void __init time_init(void)
 	timer_set_signal_handler();
 	late_time_init = um_timer_setup;
 }
+
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+__setup("virtual-time", os_setup_virtual_time);
+__uml_help(os_setup_virtual_time,
+"virtual-time\n"
+"    Run the system in virtual time mode, i.e. bump time\n"
+"    forward when there's nothing to do, rather than waiting\n"
+"    for real time to elapse. Useful for test execution.\n\n"
+);
+#endif
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index ea720149f5b8..d37ee59cb936 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -15,8 +15,27 @@
 #include <os.h>
 #include <string.h>
 #include <timer-internal.h>
+#include <generated/autoconf.h>
 
 static timer_t event_high_res_timer = 0;
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+unsigned long long virtual_time;
+unsigned long long timer_expiry;
+static enum {
+	TMR_DIS,
+	TMR_ONE,
+	TMR_INT,
+} timer_mode;
+static bool virtual_time_enabled;
+
+int os_setup_virtual_time(char *str)
+{
+	virtual_time_enabled = true;
+	return 1;
+}
+#else
+#define virtual_time_enabled false
+#endif
 
 static inline long long timeval_to_ns(const struct timeval *tv)
 {
@@ -66,6 +85,11 @@ int os_timer_set_interval(void)
 	if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
 		return -errno;
 
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	timer_mode = TMR_INT;
+	timer_expiry = virtual_time + nsec;
+#endif
+
 	return 0;
 }
 
@@ -81,6 +105,10 @@ int os_timer_one_shot(unsigned long ticks)
 	};
 
 	timer_settime(event_high_res_timer, 0, &its, NULL);
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	timer_mode = TMR_ONE;
+	timer_expiry = virtual_time + nsec;
+#endif
 	return 0;
 }
 
@@ -93,12 +121,20 @@ void os_timer_disable(void)
 
 	memset(&its, 0, sizeof(struct itimerspec));
 	timer_settime(event_high_res_timer, 0, &its, NULL);
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	timer_mode = TMR_DIS;
+#endif
 }
 
 long long os_nsecs(void)
 {
 	struct timespec ts;
 
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	if (virtual_time_enabled)
+		return virtual_time;
+#endif
+
 	clock_gettime(CLOCK_MONOTONIC,&ts);
 	return timespec_to_ns(&ts);
 }
@@ -109,6 +145,10 @@ long long os_nsecs(void)
  */
 void os_idle_sleep(unsigned long long nsecs)
 {
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	unsigned long long next = virtual_time + nsecs;
+	struct itimerspec stop = {}, cfg;
+#endif
 	struct timespec ts = {
 		.tv_sec  = nsecs / UM_NSEC_PER_SEC,
 		.tv_nsec = nsecs % UM_NSEC_PER_SEC
@@ -117,6 +157,32 @@ void os_idle_sleep(unsigned long long nsecs)
 	/*
 	 * Relay the signal if clock_nanosleep is interrupted.
 	 */
-	if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
+	if (!virtual_time_enabled) {
+		if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
+			deliver_alarm();
+		return;
+	}
+
+#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
+	timer_settime(event_high_res_timer, 0, &stop, &cfg);
+
+	if (timer_mode != TMR_DIS && timer_expiry < next) {
+		if (timer_mode == TMR_ONE)
+			timer_mode = TMR_DIS;
+		/* virtual_time will be adjusted in timer_handler() */
 		deliver_alarm();
+		return;
+	}
+
+	virtual_time = next;
+
+	if (timer_mode != TMR_DIS) {
+		unsigned long long remaining = timer_expiry - virtual_time;
+
+		cfg.it_value.tv_sec = remaining / UM_NSEC_PER_SEC;
+		cfg.it_value.tv_nsec = remaining % UM_NSEC_PER_SEC;
+
+		timer_settime(event_high_res_timer, 0, &cfg, NULL);
+	}
+#endif
 }
-- 
2.17.2


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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] arch: um: support virtual time
  2019-05-06 12:39 ` [PATCH v2 5/5] arch: um: support virtual time Johannes Berg
@ 2019-05-26 21:55   ` Richard Weinberger
  2019-05-26 22:18     ` Johannes Berg
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Weinberger @ 2019-05-26 21:55 UTC (permalink / raw)
  To: Johannes Berg; +Cc: j, linux-um, Johannes Berg

On Mon, May 6, 2019 at 2:40 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> 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_SUPPORT - to support such
> behavior; it needs to be enabled with the "virtual-time" option
> passed to the UML invocation.

I like this feature!
Is "virtual time" a common name for such a mode?
To me "virtual time" reads like a clock that runs with different speed or is in
some other way untangled from the host.
What you have implement is time traveling. ;-)

> 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/include/shared/os.h | 14 ++++++++
>  arch/um/kernel/time.c       | 13 +++++++
>  arch/um/os-Linux/time.c     | 68 ++++++++++++++++++++++++++++++++++++-
>  4 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/arch/um/Kconfig b/arch/um/Kconfig
> index ec9711d068b7..71ff7ef3aa0c 100644
> --- a/arch/um/Kconfig
> +++ b/arch/um/Kconfig
> @@ -180,6 +180,23 @@ config SECCOMP
>
>           If unsure, say Y.
>
> +config UML_VIRTUAL_TIME_SUPPORT
> +       bool
> +       prompt "Support virtual time (e.g. for test execution)"
> +       help
> +         Enable this option to support 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 may not always
> +         cause the virtual time to increase unless enough CPU was consumed to
> +         advance the tick (HZ).
> +
> +         Note that to enable the virtual time, you also need to pass
> +         "virtual-time" on the command-line.
> +
> +         It is safe to say Y, but you probably don't need this, so say N.
> +
>  endmenu
>
>  source "arch/um/drivers/Kconfig"
> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
> index 449e71edefaa..a891a5665704 100644
> --- a/arch/um/include/shared/os.h
> +++ b/arch/um/include/shared/os.h
> @@ -257,6 +257,20 @@ 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);
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +extern unsigned long long virtual_time;
> +extern unsigned long long timer_expiry;
> +int os_setup_virtual_time(char *str);
> +static inline void os_set_virtual_time_to_timer(void)
> +{
> +       /* ignored if virtual time isn't enabled */
> +       virtual_time = timer_expiry;
> +}
> +#else
> +static inline void os_set_virtual_time_to_timer(void)
> +{
> +}
> +#endif
>
>  /* 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 3898119f773e..0ceb7c540d60 100644
> --- a/arch/um/kernel/time.c
> +++ b/arch/um/kernel/time.c
> @@ -19,11 +19,14 @@
>  #include <kern_util.h>
>  #include <os.h>
>  #include <timer-internal.h>
> +#include <shared/init.h>
>
>  void timer_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
>  {
>         unsigned long flags;
>
> +       os_set_virtual_time_to_timer();
> +
>         local_irq_save(flags);
>         do_IRQ(TIMER_IRQ, regs);
>         local_irq_restore(flags);
> @@ -134,3 +137,13 @@ void __init time_init(void)
>         timer_set_signal_handler();
>         late_time_init = um_timer_setup;
>  }
> +
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +__setup("virtual-time", os_setup_virtual_time);
> +__uml_help(os_setup_virtual_time,
> +"virtual-time\n"
> +"    Run the system in virtual time mode, i.e. bump time\n"
> +"    forward when there's nothing to do, rather than waiting\n"
> +"    for real time to elapse. Useful for test execution.\n\n"
> +);
> +#endif
> diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
> index ea720149f5b8..d37ee59cb936 100644
> --- a/arch/um/os-Linux/time.c
> +++ b/arch/um/os-Linux/time.c
> @@ -15,8 +15,27 @@
>  #include <os.h>
>  #include <string.h>
>  #include <timer-internal.h>
> +#include <generated/autoconf.h>
>
>  static timer_t event_high_res_timer = 0;
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +unsigned long long virtual_time;
> +unsigned long long timer_expiry;
> +static enum {
> +       TMR_DIS,
> +       TMR_ONE,
> +       TMR_INT,
> +} timer_mode;

You set the timer mode in the os_*-functions, this works because the only user
is UML's posix-timer.
Is there a reason why you didn't install most virtual time hooks in
the itimer_* functions?
This feels more natural to me and would keep the os_*-functions
stateless and generic.

> +static bool virtual_time_enabled;
> +
> +int os_setup_virtual_time(char *str)
> +{
> +       virtual_time_enabled = true;
> +       return 1;
> +}
> +#else
> +#define virtual_time_enabled false
> +#endif
>
>  static inline long long timeval_to_ns(const struct timeval *tv)
>  {
> @@ -66,6 +85,11 @@ int os_timer_set_interval(void)
>         if (timer_settime(event_high_res_timer, 0, &its, NULL) == -1)
>                 return -errno;
>
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       timer_mode = TMR_INT;
> +       timer_expiry = virtual_time + nsec;
> +#endif
>

Can we please have a static inline helper for this?
...to avoid more ifdefs in C files.

>         return 0;
>  }
>
> @@ -81,6 +105,10 @@ int os_timer_one_shot(unsigned long ticks)
>         };
>
>         timer_settime(event_high_res_timer, 0, &its, NULL);
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       timer_mode = TMR_ONE;
> +       timer_expiry = virtual_time + nsec;
> +#endif

Same.

>         return 0;
>  }
>
> @@ -93,12 +121,20 @@ void os_timer_disable(void)
>
>         memset(&its, 0, sizeof(struct itimerspec));
>         timer_settime(event_high_res_timer, 0, &its, NULL);
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       timer_mode = TMR_DIS;
> +#endif
>  }

Same.

>  long long os_nsecs(void)
>  {
>         struct timespec ts;
>
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       if (virtual_time_enabled)
> +               return virtual_time;
> +#endif
>

Do we need the ifdef here?
If CONFIG_UML_VIRTUAL_TIME_SUPPORT is disabled, virtual_time_enabled
should never be non-zero/true.

>         clock_gettime(CLOCK_MONOTONIC,&ts);
>         return timespec_to_ns(&ts);
>  }
> @@ -109,6 +145,10 @@ long long os_nsecs(void)
>   */
>  void os_idle_sleep(unsigned long long nsecs)
>  {
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       unsigned long long next = virtual_time + nsecs;
> +       struct itimerspec stop = {}, cfg;
> +#endif
>         struct timespec ts = {
>                 .tv_sec  = nsecs / UM_NSEC_PER_SEC,
>                 .tv_nsec = nsecs % UM_NSEC_PER_SEC
> @@ -117,6 +157,32 @@ void os_idle_sleep(unsigned long long nsecs)
>         /*
>          * Relay the signal if clock_nanosleep is interrupted.
>          */
> -       if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
> +       if (!virtual_time_enabled) {
> +               if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
> +                       deliver_alarm();
> +               return;
> +       }
> +
> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> +       timer_settime(event_high_res_timer, 0, &stop, &cfg);
> +
> +       if (timer_mode != TMR_DIS && timer_expiry < next) {
> +               if (timer_mode == TMR_ONE)
> +                       timer_mode = TMR_DIS;
> +               /* virtual_time will be adjusted in timer_handler() */
>                 deliver_alarm();
> +               return;
> +       }
> +
> +       virtual_time = next;
> +
> +       if (timer_mode != TMR_DIS) {
> +               unsigned long long remaining = timer_expiry - virtual_time;
> +
> +               cfg.it_value.tv_sec = remaining / UM_NSEC_PER_SEC;
> +               cfg.it_value.tv_nsec = remaining % UM_NSEC_PER_SEC;
> +
> +               timer_settime(event_high_res_timer, 0, &cfg, NULL);
> +       }
> +#endif

Please split the function to get rid of the ifdefs.

-- 
Thanks,
//richard

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] arch: um: support virtual time
  2019-05-26 21:55   ` Richard Weinberger
@ 2019-05-26 22:18     ` Johannes Berg
  2019-05-27  4:47       ` Anton Ivanov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2019-05-26 22:18 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: j, linux-um

On Sun, 2019-05-26 at 23:55 +0200, Richard Weinberger wrote:

> > Add a mode - CONFIG_UML_VIRTUAL_TIME_SUPPORT - to support such
> > behavior; it needs to be enabled with the "virtual-time" option
> > passed to the UML invocation.
> 
> I like this feature!
> Is "virtual time" a common name for such a mode?

I have no idea, sorry. If you find any references to this being done
elsewhere, we can certainly rename it. A colleague pointed me to various
network simulation papers which play with the clock, but I don't recall
seeing a good name for this (but it's also past midnight, so ...)

> To me "virtual time" reads like a clock that runs with different speed or is in
> some other way untangled from the host.
> What you have implement is time traveling. ;-)

True :-)

I have a version of this that even implements "infinite CPU power" by
completely eliding the calls to the host timer (which lets us do
preemption). This has a large number of issues, but also found a few
bugs already, e.g.
https://github.com/bcopeland/wmediumd/commit/414bee49eda82046b61e0a3cd583d235ebd3f017

The biggest issue is that nothing actually takes time, and so things
like

https://bugs.python.org/issue37026

result. When I make just reading out the time take 10 ns or so, things
like that go away.

It still has issues, like kernel work queues won't run until userspace
is completely quiescent, which is clearly unrealistic. Trying to model
some time into the syscall now, so that can "take" some time and the
scheduler will run kernel threads, but it's not really clear to me yet
how that can be made to work. It should solve this problem though.

The reason I'm interested in that is that it completely decouples the
code from the real time, e.g. if I run a ton of debug code somewhere, it
won't affect my "timing", and thus not cause differences in the test
execution.

> > +++ b/arch/um/os-Linux/time.c
> > @@ -15,8 +15,27 @@
> >  #include <os.h>
> >  #include <string.h>
> >  #include <timer-internal.h>
> > +#include <generated/autoconf.h>
> > 
> >  static timer_t event_high_res_timer = 0;
> > +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> > +unsigned long long virtual_time;
> > +unsigned long long timer_expiry;
> > +static enum {
> > +       TMR_DIS,
> > +       TMR_ONE,
> > +       TMR_INT,
> > +} timer_mode;
> 
> You set the timer mode in the os_*-functions, this works because the only user
> is UML's posix-timer.
> Is there a reason why you didn't install most virtual time hooks in
> the itimer_* functions?
> This feels more natural to me and would keep the os_*-functions
> stateless and generic.

Can't really say I had a good reason for that. It's probably just the
place I could actually reason best about - and in the case of my non-
preemptible mode (that I described above) really be sure nothing was
calling any real timers :-)

It could totally be done in the itimer_* functions, but then also has to
be in arch_cpu_idle() I guess, to be done in the kernel side (rather
than in the host side in os_*).

> > +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> > +       timer_mode = TMR_INT;
> > +       timer_expiry = virtual_time + nsec;
> > +#endif
> > 
> 
> Can we please have a static inline helper for this?
> ...to avoid more ifdefs in C files.

Sure. Not sure it's worth putting it into a header file, but it could
be, even common code with the similar code you pointed out, declared at
the top of the C file with the other ifdefs.

> >  long long os_nsecs(void)
> >  {
> >         struct timespec ts;
> > 
> > +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> > +       if (virtual_time_enabled)
> > +               return virtual_time;
> > +#endif
> > 
> 
> Do we need the ifdef here?
> If CONFIG_UML_VIRTUAL_TIME_SUPPORT is disabled, virtual_time_enabled
> should never be non-zero/true.

Yeah, some variable are ifdef'ed out so this wouldn't compile, but if
you prefer I can remove those ifdefs and this can always be compiled.

> >  void os_idle_sleep(unsigned long long nsecs)
> >  {
> > +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> > +       unsigned long long next = virtual_time + nsecs;
> > +       struct itimerspec stop = {}, cfg;
> > +#endif
> >         struct timespec ts = {
> >                 .tv_sec  = nsecs / UM_NSEC_PER_SEC,
> >                 .tv_nsec = nsecs % UM_NSEC_PER_SEC
> > @@ -117,6 +157,32 @@ void os_idle_sleep(unsigned long long nsecs)
> >         /*
> >          * Relay the signal if clock_nanosleep is interrupted.
> >          */
> > -       if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
> > +       if (!virtual_time_enabled) {
> > +               if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
> > +                       deliver_alarm();
> > +               return;
> > +       }
> > +
> > +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
> > +       timer_settime(event_high_res_timer, 0, &stop, &cfg);
> > +
> > +       if (timer_mode != TMR_DIS && timer_expiry < next) {
> > +               if (timer_mode == TMR_ONE)
> > +                       timer_mode = TMR_DIS;
> > +               /* virtual_time will be adjusted in timer_handler() */
> >                 deliver_alarm();
> > +               return;
> > +       }
> > +
> > +       virtual_time = next;
> > +
> > +       if (timer_mode != TMR_DIS) {
> > +               unsigned long long remaining = timer_expiry - virtual_time;
> > +
> > +               cfg.it_value.tv_sec = remaining / UM_NSEC_PER_SEC;
> > +               cfg.it_value.tv_nsec = remaining % UM_NSEC_PER_SEC;
> > +
> > +               timer_settime(event_high_res_timer, 0, &cfg, NULL);
> > +       }
> > +#endif
> 
> Please split the function to get rid of the ifdefs.

Hmm. We need an ifdef there somewhere anyway.

If you dislike the ifdefs so much and wanted to keep the if in
os_nsecs() anyway maybe I should just remove the Kconfig option entirely
and just let the runtime configuration control it?

johannes


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] arch: um: support virtual time
  2019-05-26 22:18     ` Johannes Berg
@ 2019-05-27  4:47       ` Anton Ivanov
  2019-05-27  6:50         ` Johannes Berg
  0 siblings, 1 reply; 11+ messages in thread
From: Anton Ivanov @ 2019-05-27  4:47 UTC (permalink / raw)
  To: Johannes Berg, Richard Weinberger; +Cc: j, linux-um



On 5/26/19 11:18 PM, Johannes Berg wrote:
> On Sun, 2019-05-26 at 23:55 +0200, Richard Weinberger wrote:
> 
>>> Add a mode - CONFIG_UML_VIRTUAL_TIME_SUPPORT - to support such
>>> behavior; it needs to be enabled with the "virtual-time" option
>>> passed to the UML invocation.
>>
>> I like this feature!
>> Is "virtual time" a common name for such a mode?
> 
> I have no idea, sorry. If you find any references to this being done
> elsewhere, we can certainly rename it. A colleague pointed me to various
> network simulation papers which play with the clock, but I don't recall
> seeing a good name for this (but it's also past midnight, so ...)

ITIMER_VIRTUAL This timer counts down against the  user-mode  CPU  time 
consumed  by the process.  (The measurement includes CPU
  time consumed by all threads in the process.)   At  each expiration, a 
SIGVTALRM signal is generated.

This is what we used to use before we migrated to POSIX timers.

So as far as the name there is a possible name clash/term overload.

> 
>> To me "virtual time" reads like a clock that runs with different speed or is in
>> some other way untangled from the host.
>> What you have implement is time traveling. ;-)
> 
> True :-)

+1, Let's just call it time travel mode.

> 
> I have a version of this that even implements "infinite CPU power" by
> completely eliding the calls to the host timer (which lets us do
> preemption). This has a large number of issues, but also found a few
> bugs already, e.g.
> https://github.com/bcopeland/wmediumd/commit/414bee49eda82046b61e0a3cd583d235ebd3f017
> 
> The biggest issue is that nothing actually takes time, and so things
> like
> 
> https://bugs.python.org/issue37026
> 
> result. When I make just reading out the time take 10 ns or so, things
> like that go away.
> 
> It still has issues, like kernel work queues won't run until userspace
> is completely quiescent, which is clearly unrealistic. Trying to model
> some time into the syscall now, so that can "take" some time and the
> scheduler will run kernel threads, but it's not really clear to me yet
> how that can be made to work. It should solve this problem though.
> 
> The reason I'm interested in that is that it completely decouples the
> code from the real time, e.g. if I run a ton of debug code somewhere, it
> won't affect my "timing", and thus not cause differences in the test
> execution.
> 
>>> +++ b/arch/um/os-Linux/time.c
>>> @@ -15,8 +15,27 @@
>>>   #include <os.h>
>>>   #include <string.h>
>>>   #include <timer-internal.h>
>>> +#include <generated/autoconf.h>
>>>
>>>   static timer_t event_high_res_timer = 0;
>>> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
>>> +unsigned long long virtual_time;
>>> +unsigned long long timer_expiry;
>>> +static enum {
>>> +       TMR_DIS,
>>> +       TMR_ONE,
>>> +       TMR_INT,
>>> +} timer_mode;
>>
>> You set the timer mode in the os_*-functions, this works because the only user
>> is UML's posix-timer.
>> Is there a reason why you didn't install most virtual time hooks in
>> the itimer_* functions?
>> This feels more natural to me and would keep the os_*-functions
>> stateless and generic.
> 
> Can't really say I had a good reason for that. It's probably just the
> place I could actually reason best about - and in the case of my non-
> preemptible mode (that I described above) really be sure nothing was
> calling any real timers :-)
> 
> It could totally be done in the itimer_* functions, but then also has to
> be in arch_cpu_idle() I guess, to be done in the kernel side (rather
> than in the host side in os_*).
> 
>>> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
>>> +       timer_mode = TMR_INT;
>>> +       timer_expiry = virtual_time + nsec;
>>> +#endif
>>>
>>
>> Can we please have a static inline helper for this?
>> ...to avoid more ifdefs in C files.
> 
> Sure. Not sure it's worth putting it into a header file, but it could
> be, even common code with the similar code you pointed out, declared at
> the top of the C file with the other ifdefs.
> 
>>>   long long os_nsecs(void)
>>>   {
>>>          struct timespec ts;
>>>
>>> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
>>> +       if (virtual_time_enabled)
>>> +               return virtual_time;
>>> +#endif
>>>
>>
>> Do we need the ifdef here?
>> If CONFIG_UML_VIRTUAL_TIME_SUPPORT is disabled, virtual_time_enabled
>> should never be non-zero/true.
> 
> Yeah, some variable are ifdef'ed out so this wouldn't compile, but if
> you prefer I can remove those ifdefs and this can always be compiled.
> 
>>>   void os_idle_sleep(unsigned long long nsecs)
>>>   {
>>> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
>>> +       unsigned long long next = virtual_time + nsecs;
>>> +       struct itimerspec stop = {}, cfg;
>>> +#endif
>>>          struct timespec ts = {
>>>                  .tv_sec  = nsecs / UM_NSEC_PER_SEC,
>>>                  .tv_nsec = nsecs % UM_NSEC_PER_SEC
>>> @@ -117,6 +157,32 @@ void os_idle_sleep(unsigned long long nsecs)
>>>          /*
>>>           * Relay the signal if clock_nanosleep is interrupted.
>>>           */
>>> -       if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
>>> +       if (!virtual_time_enabled) {
>>> +               if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
>>> +                       deliver_alarm();
>>> +               return;
>>> +       }
>>> +
>>> +#ifdef CONFIG_UML_VIRTUAL_TIME_SUPPORT
>>> +       timer_settime(event_high_res_timer, 0, &stop, &cfg);
>>> +
>>> +       if (timer_mode != TMR_DIS && timer_expiry < next) {
>>> +               if (timer_mode == TMR_ONE)
>>> +                       timer_mode = TMR_DIS;
>>> +               /* virtual_time will be adjusted in timer_handler() */
>>>                  deliver_alarm();
>>> +               return;
>>> +       }
>>> +
>>> +       virtual_time = next;
>>> +
>>> +       if (timer_mode != TMR_DIS) {
>>> +               unsigned long long remaining = timer_expiry - virtual_time;
>>> +
>>> +               cfg.it_value.tv_sec = remaining / UM_NSEC_PER_SEC;
>>> +               cfg.it_value.tv_nsec = remaining % UM_NSEC_PER_SEC;
>>> +
>>> +               timer_settime(event_high_res_timer, 0, &cfg, NULL);
>>> +       }
>>> +#endif
>>
>> Please split the function to get rid of the ifdefs.
> 
> Hmm. We need an ifdef there somewhere anyway.
> 
> If you dislike the ifdefs so much and wanted to keep the if in
> os_nsecs() anyway maybe I should just remove the Kconfig option entirely
> and just let the runtime configuration control it?
> 
> johannes
> 
> 
> _______________________________________________
> linux-um mailing list
> linux-um@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um
> 

-- 
Anton R Ivanov

http://www.kot-begemot.co.uk

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] arch: um: support virtual time
  2019-05-27  4:47       ` Anton Ivanov
@ 2019-05-27  6:50         ` Johannes Berg
  2019-05-27  7:34           ` Anton Ivanov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2019-05-27  6:50 UTC (permalink / raw)
  To: Anton Ivanov, Richard Weinberger; +Cc: j, linux-um

On Mon, 2019-05-27 at 05:47 +0100, Anton Ivanov wrote:
> ITIMER_VIRTUAL This timer counts down against the  user-mode  CPU  time 
> consumed  by the process.  (The measurement includes CPU
>   time consumed by all threads in the process.)   At  each expiration, a 
> SIGVTALRM signal is generated.
> 
> This is what we used to use before we migrated to POSIX timers.
> 
> So as far as the name there is a possible name clash/term overload.

Good point.

> > 
> > > To me "virtual time" reads like a clock that runs with different speed or is in
> > > some other way untangled from the host.
> > > What you have implement is time traveling. ;-)
> > 
> > True :-)
> 
> +1, Let's just call it time travel mode.

:-)

Checking those references from my colleague now, I see the terms
 * (adaptive) time dilation - at least for the other case I haven't
   included in this patch yet where the clock can run slower than real
   time
 * virtual time - which is closest to what I called "infinite CPU power"
   before
 * relativistic time - which is close but not really what I implemented
   here either

But sure, let's just call it "time-travel".

I think I'll add a few options:

 * time-travel=faster - what's implemented by this patch
 * time-travel=infcpu - infinite CPU power available
 * time-travel-start=<int value>
                      - start of real time, to not necessarily use wall
                        clock

Seems reasonable?

johannes


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] arch: um: support virtual time
  2019-05-27  6:50         ` Johannes Berg
@ 2019-05-27  7:34           ` Anton Ivanov
  0 siblings, 0 replies; 11+ messages in thread
From: Anton Ivanov @ 2019-05-27  7:34 UTC (permalink / raw)
  To: Johannes Berg, Richard Weinberger; +Cc: j, linux-um



On 27/05/2019 07:50, Johannes Berg wrote:
> On Mon, 2019-05-27 at 05:47 +0100, Anton Ivanov wrote:
>> ITIMER_VIRTUAL This timer counts down against the  user-mode  CPU  time
>> consumed  by the process.  (The measurement includes CPU
>>    time consumed by all threads in the process.)   At  each expiration, a
>> SIGVTALRM signal is generated.
>>
>> This is what we used to use before we migrated to POSIX timers.
>>
>> So as far as the name there is a possible name clash/term overload.
> 
> Good point.
> 
>>>
>>>> To me "virtual time" reads like a clock that runs with different speed or is in
>>>> some other way untangled from the host.
>>>> What you have implement is time traveling. ;-)
>>>
>>> True :-)
>>
>> +1, Let's just call it time travel mode.
> 
> :-)
> 
> Checking those references from my colleague now, I see the terms
>   * (adaptive) time dilation - at least for the other case I haven't
>     included in this patch yet where the clock can run slower than real
>     time
>   * virtual time - which is closest to what I called "infinite CPU power"
>     before
>   * relativistic time - which is close but not really what I implemented
>     here either
> 
> But sure, let's just call it "time-travel".
> 
> I think I'll add a few options:
> 
>   * time-travel=faster - what's implemented by this patch
>   * time-travel=infcpu - infinite CPU power available
>   * time-travel-start=<int value>
>                        - start of real time, to not necessarily use wall
>                          clock
> 
> Seems reasonable?

Yes. It also clearly distinguishes it from virtual time which is 
something which is used in a few places as a term.

Best Regards,


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

-- 
Anton R Ivanov

http://www.kot-begemot.co.uk

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-27  7:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-06 12:39 [PATCH v2 0/5] arch: um: fixes and virtual time support Johannes Berg
2019-05-06 12:39 ` [PATCH v2 1/5] arch: um: Fix IRQ controller regression on console read Johannes Berg
2019-05-06 12:39 ` [PATCH v2 2/5] arch: um: use cpu_possible_mask to avoid warning Johannes Berg
2019-05-06 12:39 ` [PATCH v2 3/5] arch: um: fix os_timer_one_shot() Johannes Berg
2019-05-06 12:39 ` [PATCH v2 4/5] arch: um: timer code cleanup Johannes Berg
2019-05-06 12:39 ` [PATCH v2 5/5] arch: um: support virtual time Johannes Berg
2019-05-26 21:55   ` Richard Weinberger
2019-05-26 22:18     ` Johannes Berg
2019-05-27  4:47       ` Anton Ivanov
2019-05-27  6:50         ` Johannes Berg
2019-05-27  7:34           ` Anton Ivanov

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.