public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Thiemo Seufer <ths-eH4hzgmiRX8dwXzzRB9H2Q@public.gmane.org>
To: Luca Tettamanti <kronos.it-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Dan Kenigsberg <dank-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
	qemu-devel-qX2TKyscuCcdnm+yROfE0A@public.gmane.org
Subject: Re: [Qemu-devel] Re:  [PATCH] Dynamic ticks
Date: Tue, 14 Aug 2007 00:06:20 +0100	[thread overview]
Message-ID: <20070813230620.GB21215@networkno.de> (raw)
In-Reply-To: <20070813203741.GA20747-sTXFmx6KbOnUXq0IF5SVAZ4oGUkBHcCu@public.gmane.org>

Luca Tettamanti wrote:
[snip]
> I've implemented some of my suggestions in the following patch - rebased
> to kvm-userspace current git since it's easier to test (...ok, I'm lazy -
> but you get the idea):
> 
> 
> diff --git a/qemu/configure b/qemu/configure
> index 365b7fb..38373db 100755
> --- a/qemu/configure
> +++ b/qemu/configure
> @@ -262,6 +262,8 @@ for opt do
>    ;;
>    --enable-uname-release=*) uname_release="$optarg"
>    ;;
> +  --disable-dynamic-ticks) dynamic_ticks="no"
> +  ;;

Is there a situation where the attempt to use dynticks is harmful?

>    esac
>  done
>  
> @@ -788,6 +790,9 @@ echo "TARGET_DIRS=$target_list" >> $config_mak
>  if [ "$build_docs" = "yes" ] ; then
>    echo "BUILD_DOCS=yes" >> $config_mak
>  fi
> +if test "$dynamic_ticks" != "no" ; then
> +  echo "#define DYNAMIC_TICKS 1" >> $config_h
> +fi
>  
>  # XXX: suppress that
>  if [ "$bsd" = "yes" ] ; then
> diff --git a/qemu/vl.c b/qemu/vl.c
> index 4b73f77..05639d7 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -759,6 +759,35 @@ static unsigned int period = 1;
>  static int timer_freq;
>  #endif
>  
> +#ifdef DYNAMIC_TICKS
> +/* If DYNAMIC_TICKS is defined (and use_dynamic_ticks selected) qemu does not
> + * attepmt to generate SIGALRM at a constant rate. Rather, the system timer is
> + * set to generate SIGALRM only when it is needed. DYNAMIC_TICKS reduces the
> + * number of SIGALRMs sent to idle dynamic-ticked guests. */
> +
> +static void rearm_host_timer(void);
> +static int dynticks_create_timer(void);
> +
> +static int use_dynticks = 1;
> +
> +static int use_dynamic_ticks() {
> +    return use_dynticks;
> +}
> +
> +#else
> +
> +static void rearm_host_timer(void) {}
> +
> +static int dynticks_create_timer(void) {
> +    return 0;
> +}
> +
> +static int use_dynamic_ticks() {
> +    return 0;
> +}
> +
> +#endif

This optimization is probably not worth an #ifdef.

> +
>  QEMUClock *qemu_new_clock(int type)
>  {
>      QEMUClock *clock;
> @@ -803,6 +832,7 @@ void qemu_del_timer(QEMUTimer *ts)
>          }
>          pt = &t->next;
>      }
> +    rearm_host_timer();
>  }
>  
>  /* modify the current timer so that it will be fired when current_time
> @@ -862,6 +892,7 @@ static void qemu_run_timers(QEMUTimer **ptimer_head, int64_t current_time)
>          /* run the callback (the timer list can be modified) */
>          ts->cb(ts->opaque);
>      }
> +    rearm_host_timer();
>  }
>  
>  int64_t qemu_get_clock(QEMUClock *clock)
> @@ -969,7 +1000,8 @@ static void host_alarm_handler(int host_signum)
>          last_clock = ti;
>      }
>  #endif
> -    if (qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
> +    if (use_dynamic_ticks() ||
> +        qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
>                             qemu_get_clock(vm_clock)) ||
>          qemu_timer_expired(active_timers[QEMU_TIMER_REALTIME],
>                             qemu_get_clock(rt_clock))) {
> @@ -1126,21 +1158,26 @@ static void init_timer_alarm(void)
>          act.sa_handler = host_alarm_handler;
>          sigaction(SIGALRM, &act, NULL);
>  
> -        itv.it_interval.tv_sec = 0;
> -        itv.it_interval.tv_usec = 999; /* for i386 kernel 2.6 to get 1 ms */
> -        itv.it_value.tv_sec = 0;
> -        itv.it_value.tv_usec = 10 * 1000;
> -        setitimer(ITIMER_REAL, &itv, NULL);
> -        /* we probe the tick duration of the kernel to inform the user if
> -           the emulated kernel requested a too high timer frequency */
> -        getitimer(ITIMER_REAL, &itv);
> +        if (use_dynamic_ticks() && dynticks_create_timer()) {

Maybe

dynticks_create_timer();
if (!use_dynamic_ticks()) {
   ...

is a bit clearer. Or maybe put both alternatives in a create_timer()
function.

> +            /* dynticks disabled or failed to create a timer, fallback
> +             * to old code.
> +             */
> +            itv.it_interval.tv_sec = 0;
> +            itv.it_interval.tv_usec = 999; /* for i386 kernel 2.6 to get 1 ms */
> +            itv.it_value.tv_sec = 0;
> +            itv.it_value.tv_usec = 10 * 1000;
> +            setitimer(ITIMER_REAL, &itv, NULL);
> +            /* we probe the tick duration of the kernel to inform the user if
> +               the emulated kernel requested a too high timer frequency */
> +            getitimer(ITIMER_REAL, &itv);
> +        }
>  
>  #if defined(__linux__)
>          /* XXX: force /dev/rtc usage because even 2.6 kernels may not
>             have timers with 1 ms resolution. The correct solution will
>             be to use the POSIX real time timers available in recent
>             2.6 kernels */
> -        if (itv.it_interval.tv_usec > 1000 || 1) {
> +        if (!use_dynamic_ticks() && (itv.it_interval.tv_usec > 1000 || 1)) {
>              /* try to use /dev/rtc to have a faster timer */
>              if ((!use_rtc && !use_hpet) || (start_rtc_timer() < 0))
>                  goto use_itimer;
> @@ -6110,6 +6147,7 @@ void vm_start(void)
>          cpu_enable_ticks();
>          vm_running = 1;
>          vm_state_notify(1);
> +        rearm_host_timer();
>      }
>  }
>  
> @@ -6536,6 +6574,9 @@ void help(void)
>             "-no-rtc         don't use /dev/rtc for timer alarm (do use gettimeofday)\n"
>             "-use-hpet       use /dev/hpet (HPET) instead of RTC for timer alarm\n"
>  #endif
> +#ifdef DYNAMIC_TICKS
> +           "-no-dyntick     don't use dynamic ticks\n"
> +#endif
>  	   "-option-rom rom load a file, rom, into the option ROM space\n"
>             "\n"
>             "During emulation, the following keys are useful:\n"
> @@ -6630,6 +6671,9 @@ enum {
>      QEMU_OPTION_use_hpet,
>  #endif
>      QEMU_OPTION_cpu_vendor,
> +#ifdef DYNAMIC_TICKS
> +    QEMU_OPTION_no_dyntick,
> +#endif
>  };
>  
>  typedef struct QEMUOption {
> @@ -6728,6 +6772,9 @@ const QEMUOption qemu_options[] = {
>      { "use-hpet", 0, QEMU_OPTION_use_hpet },
>  #endif
>      { "cpu-vendor", HAS_ARG, QEMU_OPTION_cpu_vendor },
> +#if defined(DYNAMIC_TICKS)
> +    { "no-dyntick", 0, QEMU_OPTION_no_dyntick },
> +#endif
>      { NULL },
>  };
>  
> @@ -6932,6 +6979,78 @@ static BOOL WINAPI qemu_ctrl_handler(DWORD type)
>  
>  #define MAX_NET_CLIENTS 32
>  
> +#ifdef DYNAMIC_TICKS
> +
> +static timer_t host_timer;
> +
> +static int dynticks_create_timer() {
> +    struct sigevent ev;
> +
> +    ev.sigev_value.sival_int = 0;
> +    ev.sigev_notify = SIGEV_SIGNAL;
> +    ev.sigev_signo = SIGALRM;
> +
> +    if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) {
> +        perror("timer_create");
> +
> +        /* disable dynticks */
> +        fprintf(stderr, "Dynamic Ticks disabled\n");
> +        use_dynticks = 0;
> +
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/* call host_alarm_handler just when the nearest QEMUTimer expires */
> +/* expire_time is measured in nanosec for vm_clock but in millisec
> + * for rt_clock
> + */
> +static void rearm_host_timer(void) {
> +    struct itimerspec timeout;
> +    int64_t nearest_delta_us = INT64_MAX;
> +
> +    if (!use_dynamic_ticks())
> +        return;
> +
> +    if (active_timers[QEMU_TIMER_REALTIME] ||
> +            active_timers[QEMU_TIMER_VIRTUAL]) {
> +        int64_t vmdelta_us, current_us;
> +
> +        if (active_timers[QEMU_TIMER_REALTIME])
> +            nearest_delta_us = (active_timers[QEMU_TIMER_REALTIME]->expire_time - qemu_get_clock(rt_clock))*1000;
> +
> +        if (active_timers[QEMU_TIMER_VIRTUAL]) {
> +            /* round up */
> +            vmdelta_us = (active_timers[QEMU_TIMER_VIRTUAL]->expire_time - qemu_get_clock(vm_clock)+999)/1000;
> +            if (vmdelta_us < nearest_delta_us)
> +                nearest_delta_us = vmdelta_us;
> +        }
> +
> +        /* Avoid arming the timer to negative, zero, or too low values */
> +        /* MIN_TIMER_REARM_US should be optimized */
> +#define MIN_TIMER_REARM_US 250
> +        if (nearest_delta_us <= MIN_TIMER_REARM_US)
> +            nearest_delta_us = MIN_TIMER_REARM_US;
> +
> +        /* check whether a timer is already running */
> +        if (timer_gettime(host_timer, &timeout))
> +            perror("gettime");

... and exit() ?

> +        current_us = timeout.it_value.tv_sec * 1000000 + timeout.it_value.tv_nsec/1000;
> +        if (current_us && current_us <= nearest_delta_us)
> +            return;
> +
> +        timeout.it_interval.tv_sec = 0;
> +        timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */
> +        timeout.it_value.tv_sec =  nearest_delta_us / 1000000;
> +        timeout.it_value.tv_nsec = nearest_delta_us % 1000000 * 1000;
> +        if (timer_settime(host_timer, 0 /* RELATIVE */, &timeout, NULL))
> +            perror("settime");

Likewise here.

> +    }
> +}
> +#endif /* DYNAMIC_TICKS */
> +
>  static int saved_argc;
>  static char **saved_argv;
>  
> @@ -7457,6 +7576,11 @@ int main(int argc, char **argv)
>  	    case QEMU_OPTION_cpu_vendor:
>  		cpu_vendor_string = optarg;
>  		break;
> +#ifdef DYNAMIC_TICKS
> +                case QEMU_OPTION_no_dyntick:
> +                use_dynticks = 0;
> +                break;
> +#endif
>              }
>          }
>      }
> 
> Luca
> -- 
> Se non puoi convincerli, confondili.
> 
> 
> 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

  parent reply	other threads:[~2007-08-13 23:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-13 14:42 [PATCH] Dynamic ticks Dan Kenigsberg
2007-08-13 20:37 ` [kvm-devel] " Luca Tettamanti
     [not found]   ` <20070813203741.GA20747-sTXFmx6KbOnUXq0IF5SVAZ4oGUkBHcCu@public.gmane.org>
2007-08-13 23:06     ` Thiemo Seufer [this message]
     [not found]       ` <20070813230620.GB21215-eH4hzgmiRX8dwXzzRB9H2Q@public.gmane.org>
2007-08-15 17:05         ` Dan Kenigsberg
2007-08-15 17:13     ` Dan Kenigsberg
     [not found]       ` <20070815171348.GB15482-RO/WWmT55CHJJbofclyLPCHBx9XpghdU@public.gmane.org>
2007-08-15 19:48         ` Dor Laor
2007-08-16  9:36     ` Dan Kenigsberg

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=20070813230620.GB21215@networkno.de \
    --to=ths-eh4hzgmirx8dwxzzrb9h2q@public.gmane.org \
    --cc=dank-atKUWr5tajBWk0Htik3J/w@public.gmane.org \
    --cc=kronos.it-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    --cc=qemu-devel-qX2TKyscuCcdnm+yROfE0A@public.gmane.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox