Re: [Qemu-devel] Re: [kvm-devel] [PATCH] Dynamic ticks

[Thiemo Seufer]

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.
> 
> 
>