diff mbox

[v2,2/3] kvm tools: remove periodic tick in favour of a polling thread

Message ID 1378301148-18823-3-git-send-email-jonathan.austin@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jonathan Austin Sept. 4, 2013, 1:25 p.m. UTC 
Currently the only use of the periodic timer tick in kvmtool is to
handle reading from stdin. Though functional, this periodic tick can be
problematic on slow (eg FPGA) platforms and can cause low interactivity or
even stop the execution from progressing at all.

This patch removes the periodic tick in favour of a dedicated thread blocked
waiting for input from the console. In order to reflect the new behaviour,
the old 'kvm__arch_periodic_tick' function is renamed to 'kvm__arch_read_term'.

Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/arm/kvm.c         |    2 +-
 tools/kvm/builtin-run.c     |   13 -----------
 tools/kvm/include/kvm/kvm.h |    2 +-
 tools/kvm/kvm.c             |   50 -------------------------------------------
 tools/kvm/powerpc/kvm.c     |    2 +-
 tools/kvm/term.c            |   31 +++++++++++++++++++++++++++
 tools/kvm/x86/kvm.c         |    2 +-
 7 files changed, 35 insertions(+), 67 deletions(-)

Comments

Pekka Enberg Sept. 4, 2013, 3:58 p.m. UTC | #1 
Hi Jonathan,

On Wed, Sep 4, 2013 at 4:25 PM, Jonathan Austin <jonathan.austin@arm.com> wrote:
> Currently the only use of the periodic timer tick in kvmtool is to
> handle reading from stdin. Though functional, this periodic tick can be
> problematic on slow (eg FPGA) platforms and can cause low interactivity or
> even stop the execution from progressing at all.
>
> This patch removes the periodic tick in favour of a dedicated thread blocked
> waiting for input from the console. In order to reflect the new behaviour,
> the old 'kvm__arch_periodic_tick' function is renamed to 'kvm__arch_read_term'.
>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>

I'm afraid this breaks "top" on x86. Does it work on arm?

When I start it up, it seems as if it's stuck but whenever I press a
key, it prints
part of the screen.

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index 27e6cf4..008b7fe 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -46,7 +46,7 @@  void kvm__arch_delete_ram(struct kvm *kvm)
 	munmap(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size);
 }
 
-void kvm__arch_periodic_poll(struct kvm *kvm)
+void kvm__arch_read_term(struct kvm *kvm)
 {
 	if (term_readable(0)) {
 		serial8250__update_consoles(kvm);
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 4d7fbf9d..da95d71 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -165,13 +165,6 @@  void kvm_run_set_wrapper_sandbox(void)
 	OPT_END()							\
 	};
 
-static void handle_sigalrm(int sig, siginfo_t *si, void *uc)
-{
-	struct kvm *kvm = si->si_value.sival_ptr;
-
-	kvm__arch_periodic_poll(kvm);
-}
-
 static void *kvm_cpu_thread(void *arg)
 {
 	char name[16];
@@ -487,17 +480,11 @@  static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 {
 	static char real_cmdline[2048], default_name[20];
 	unsigned int nr_online_cpus;
-	struct sigaction sa;
 	struct kvm *kvm = kvm__new();
 
 	if (IS_ERR(kvm))
 		return kvm;
 
-	sa.sa_flags = SA_SIGINFO;
-	sa.sa_sigaction = handle_sigalrm;
-	sigemptyset(&sa.sa_mask);
-	sigaction(SIGALRM, &sa, NULL);
-
 	nr_online_cpus = sysconf(_SC_NPROCESSORS_ONLN);
 	kvm->cfg.custom_rootfs_name = "default";
 
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index ad53ca7..d05b936 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -103,7 +103,7 @@  void kvm__arch_delete_ram(struct kvm *kvm);
 int kvm__arch_setup_firmware(struct kvm *kvm);
 int kvm__arch_free_firmware(struct kvm *kvm);
 bool kvm__arch_cpu_supports_vm(void);
-void kvm__arch_periodic_poll(struct kvm *kvm);
+void kvm__arch_read_term(struct kvm *kvm);
 
 void *guest_flat_to_host(struct kvm *kvm, u64 offset);
 u64 host_to_guest_flat(struct kvm *kvm, void *ptr);
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index cfd30dd..d7d2e84 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -393,56 +393,6 @@  found_kernel:
 	return ret;
 }
 
-#define TIMER_INTERVAL_NS 1000000	/* 1 msec */
-
-/*
- * This function sets up a timer that's used to inject interrupts from the
- * userspace hypervisor into the guest at periodical intervals. Please note
- * that clock interrupt, for example, is not handled here.
- */
-int kvm_timer__init(struct kvm *kvm)
-{
-	struct itimerspec its;
-	struct sigevent sev;
-	int r;
-
-	memset(&sev, 0, sizeof(struct sigevent));
-	sev.sigev_value.sival_int	= 0;
-	sev.sigev_notify		= SIGEV_THREAD_ID;
-	sev.sigev_signo			= SIGALRM;
-	sev.sigev_value.sival_ptr	= kvm;
-	sev._sigev_un._tid		= syscall(__NR_gettid);
-
-	r = timer_create(CLOCK_REALTIME, &sev, &kvm->timerid);
-	if (r < 0)
-		return r;
-
-	its.it_value.tv_sec		= TIMER_INTERVAL_NS / 1000000000;
-	its.it_value.tv_nsec		= TIMER_INTERVAL_NS % 1000000000;
-	its.it_interval.tv_sec		= its.it_value.tv_sec;
-	its.it_interval.tv_nsec		= its.it_value.tv_nsec;
-
-	r = timer_settime(kvm->timerid, 0, &its, NULL);
-	if (r < 0) {
-		timer_delete(kvm->timerid);
-		return r;
-	}
-
-	return 0;
-}
-firmware_init(kvm_timer__init);
-
-int kvm_timer__exit(struct kvm *kvm)
-{
-	if (kvm->timerid)
-		if (timer_delete(kvm->timerid) < 0)
-			die("timer_delete()");
-
-	kvm->timerid = 0;
-
-	return 0;
-}
-firmware_exit(kvm_timer__exit);
 
 void kvm__dump_mem(struct kvm *kvm, unsigned long addr, unsigned long size, int debug_fd)
 {
diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
index b4b9f82..c1712cf 100644
--- a/tools/kvm/powerpc/kvm.c
+++ b/tools/kvm/powerpc/kvm.c
@@ -151,7 +151,7 @@  void kvm__irq_trigger(struct kvm *kvm, int irq)
 	kvm__irq_line(kvm, irq, 0);
 }
 
-void kvm__arch_periodic_poll(struct kvm *kvm)
+void kvm__arch_read_term(struct kvm *kvm)
 {
 	/* FIXME: Should register callbacks to platform-specific polls */
 	spapr_hvcons_poll(kvm);
diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index ac9c7cc..5c3e543 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -25,6 +25,8 @@  bool term_got_escape	= false;
 
 int term_fds[TERM_MAX_DEVS][2];
 
+static pthread_t term_poll_thread;
+
 int term_getc(struct kvm *kvm, int term)
 {
 	unsigned char c;
@@ -91,6 +93,30 @@  bool term_readable(int term)
 	return poll(&pollfd, 1, 0) > 0;
 }
 
+static void *term_poll_thread_loop(void *param)
+{
+	struct pollfd fds[TERM_MAX_DEVS];
+	struct kvm *kvm = (struct kvm *) param;
+
+	int i;
+
+	for (i = 0; i < TERM_MAX_DEVS; i++) {
+		fds[i].fd = term_fds[i][TERM_FD_IN];
+		fds[i].events = POLLIN;
+		fds[i].revents = 0;
+	}
+
+	while (1) {
+		/* Poll with infinite timeout */
+		if(poll(fds, TERM_MAX_DEVS, -1) < 1)
+			break;
+		kvm__arch_read_term(kvm);
+	}
+
+	die("term_poll_thread_loop: error polling device fds %d\n", errno);
+	return NULL;
+}
+
 static void term_cleanup(void)
 {
 	int i;
@@ -161,6 +187,11 @@  int term_init(struct kvm *kvm)
 	term.c_lflag &= ~(ICANON | ECHO | ISIG);
 	tcsetattr(STDIN_FILENO, TCSANOW, &term);
 
+
+	/* Use our own blocking thread to read stdin, don't require a tick */
+	if(pthread_create(&term_poll_thread, NULL, term_poll_thread_loop,kvm))
+		die("Unable to create console input poll thread\n");
+
 	signal(SIGTERM, term_sig_cleanup);
 	atexit(term_cleanup);
 
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index 9b46772..d285d1d 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -374,7 +374,7 @@  int kvm__arch_free_firmware(struct kvm *kvm)
 	return 0;
 }
 
-void kvm__arch_periodic_poll(struct kvm *kvm)
+void kvm__arch_read_term(struct kvm *kvm)
 {
 	serial8250__update_consoles(kvm);
 	virtio_console__inject_interrupt(kvm);