| Message ID | 165355471287.533615.6762485479325805298.stgit@pasha-ThinkPad-X280 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Record/replay refactoring and stuff | expand |
On 5/26/22 10:45, Pavel Dovgalyuk wrote: > vCPU execution should be suspended when new BH is scheduled. > This is needed to avoid guest timeouts caused by the long cycles > of the execution. In replay mode execution may hang when > vCPU sleeps and block event comes to the queue. > This patch adds notification which wakes up vCPU or interrupts > execution of guest code. > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> At least aio_bh_schedule_oneshot_full should have the same effect, so should this be done at a lower level, in aio_bh_enqueue() or even aio_notify()? Paolo > -- > > v2: changed first_cpu to current_cpu (suggested by Richard Henderson) > --- > include/sysemu/cpu-timers.h | 1 + > softmmu/icount.c | 8 ++++++++ > stubs/icount.c | 4 ++++ > util/async.c | 8 ++++++++ > 4 files changed, 21 insertions(+) > > diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h > index ed6ee5c46c..2e786fe7fb 100644 > --- a/include/sysemu/cpu-timers.h > +++ b/include/sysemu/cpu-timers.h > @@ -59,6 +59,7 @@ int64_t icount_round(int64_t count); > /* if the CPUs are idle, start accounting real time to virtual clock. */ > void icount_start_warp_timer(void); > void icount_account_warp_timer(void); > +void icount_notify_exit(void); > > /* > * CPU Ticks and Clock > diff --git a/softmmu/icount.c b/softmmu/icount.c > index 5ca271620d..1cafec5014 100644 > --- a/softmmu/icount.c > +++ b/softmmu/icount.c > @@ -486,3 +486,11 @@ void icount_configure(QemuOpts *opts, Error **errp) > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > NANOSECONDS_PER_SECOND / 10); > } > + > +void icount_notify_exit(void) > +{ > + if (icount_enabled() && current_cpu) { > + qemu_cpu_kick(current_cpu); > + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); > + } > +} > diff --git a/stubs/icount.c b/stubs/icount.c > index f13c43568b..6df8c2bf7d 100644 > --- a/stubs/icount.c > +++ b/stubs/icount.c > @@ -43,3 +43,7 @@ void icount_account_warp_timer(void) > { > abort(); > } > + > +void icount_notify_exit(void) > +{ > +} > diff --git a/util/async.c b/util/async.c > index 554ba70cca..75f50f47c4 100644 > --- a/util/async.c > +++ b/util/async.c > @@ -33,6 +33,7 @@ > #include "block/raw-aio.h" > #include "qemu/coroutine_int.h" > #include "qemu/coroutine-tls.h" > +#include "sysemu/cpu-timers.h" > #include "trace.h" > > /***********************************************************/ > @@ -185,6 +186,13 @@ void qemu_bh_schedule_idle(QEMUBH *bh) > void qemu_bh_schedule(QEMUBH *bh) > { > aio_bh_enqueue(bh, BH_SCHEDULED); > + /* > + * Workaround for record/replay. > + * vCPU execution should be suspended when new BH is set. > + * This is needed to avoid guest timeouts caused > + * by the long cycles of the execution. > + */ > + icount_notify_exit(); > } > > /* This func is async. > >
On 26.05.2022 12:37, Paolo Bonzini wrote: > On 5/26/22 10:45, Pavel Dovgalyuk wrote: >> vCPU execution should be suspended when new BH is scheduled. >> This is needed to avoid guest timeouts caused by the long cycles >> of the execution. In replay mode execution may hang when >> vCPU sleeps and block event comes to the queue. >> This patch adds notification which wakes up vCPU or interrupts >> execution of guest code. >> >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > At least aio_bh_schedule_oneshot_full should have the same effect, so > should this be done at a lower level, in aio_bh_enqueue() or even > aio_notify()? Not sure about aio_notify. It can operate with different contexts. Can some of them be not related to the VM state? Moving the notification to aio_bh_enqueue. > > Paolo > >> -- >> >> v2: changed first_cpu to current_cpu (suggested by Richard Henderson) >> --- >> include/sysemu/cpu-timers.h | 1 + >> softmmu/icount.c | 8 ++++++++ >> stubs/icount.c | 4 ++++ >> util/async.c | 8 ++++++++ >> 4 files changed, 21 insertions(+) >> >> diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h >> index ed6ee5c46c..2e786fe7fb 100644 >> --- a/include/sysemu/cpu-timers.h >> +++ b/include/sysemu/cpu-timers.h >> @@ -59,6 +59,7 @@ int64_t icount_round(int64_t count); >> /* if the CPUs are idle, start accounting real time to virtual >> clock. */ >> void icount_start_warp_timer(void); >> void icount_account_warp_timer(void); >> +void icount_notify_exit(void); >> /* >> * CPU Ticks and Clock >> diff --git a/softmmu/icount.c b/softmmu/icount.c >> index 5ca271620d..1cafec5014 100644 >> --- a/softmmu/icount.c >> +++ b/softmmu/icount.c >> @@ -486,3 +486,11 @@ void icount_configure(QemuOpts *opts, Error **errp) >> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + >> NANOSECONDS_PER_SECOND / 10); >> } >> + >> +void icount_notify_exit(void) >> +{ >> + if (icount_enabled() && current_cpu) { >> + qemu_cpu_kick(current_cpu); >> + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); >> + } >> +} >> diff --git a/stubs/icount.c b/stubs/icount.c >> index f13c43568b..6df8c2bf7d 100644 >> --- a/stubs/icount.c >> +++ b/stubs/icount.c >> @@ -43,3 +43,7 @@ void icount_account_warp_timer(void) >> { >> abort(); >> } >> + >> +void icount_notify_exit(void) >> +{ >> +} >> diff --git a/util/async.c b/util/async.c >> index 554ba70cca..75f50f47c4 100644 >> --- a/util/async.c >> +++ b/util/async.c >> @@ -33,6 +33,7 @@ >> #include "block/raw-aio.h" >> #include "qemu/coroutine_int.h" >> #include "qemu/coroutine-tls.h" >> +#include "sysemu/cpu-timers.h" >> #include "trace.h" >> /***********************************************************/ >> @@ -185,6 +186,13 @@ void qemu_bh_schedule_idle(QEMUBH *bh) >> void qemu_bh_schedule(QEMUBH *bh) >> { >> aio_bh_enqueue(bh, BH_SCHEDULED); >> + /* >> + * Workaround for record/replay. >> + * vCPU execution should be suspended when new BH is set. >> + * This is needed to avoid guest timeouts caused >> + * by the long cycles of the execution. >> + */ >> + icount_notify_exit(); >> } >> /* This func is async. >> >> >
On 5/26/22 11:51, Pavel Dovgalyuk wrote: >> >> At least aio_bh_schedule_oneshot_full should have the same effect, so >> should this be done at a lower level, in aio_bh_enqueue() or even >> aio_notify()? > > Not sure about aio_notify. It can operate with different contexts. > Can some of them be not related to the VM state? All but the main AioContext one would have current_cpu == NULL. Paolo > Moving the notification to aio_bh_enqueue.
On 26.05.2022 15:10, Paolo Bonzini wrote: > On 5/26/22 11:51, Pavel Dovgalyuk wrote: >>> >>> At least aio_bh_schedule_oneshot_full should have the same effect, so >>> should this be done at a lower level, in aio_bh_enqueue() or even >>> aio_notify()? >> >> Not sure about aio_notify. It can operate with different contexts. >> Can some of them be not related to the VM state? > > All but the main AioContext one would have current_cpu == NULL. aio_bh_enqueue is better. Moving this code to aio_notify breaks the tests.
diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h index ed6ee5c46c..2e786fe7fb 100644 --- a/include/sysemu/cpu-timers.h +++ b/include/sysemu/cpu-timers.h @@ -59,6 +59,7 @@ int64_t icount_round(int64_t count); /* if the CPUs are idle, start accounting real time to virtual clock. */ void icount_start_warp_timer(void); void icount_account_warp_timer(void); +void icount_notify_exit(void); /* * CPU Ticks and Clock diff --git a/softmmu/icount.c b/softmmu/icount.c index 5ca271620d..1cafec5014 100644 --- a/softmmu/icount.c +++ b/softmmu/icount.c @@ -486,3 +486,11 @@ void icount_configure(QemuOpts *opts, Error **errp) qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + NANOSECONDS_PER_SECOND / 10); } + +void icount_notify_exit(void) +{ + if (icount_enabled() && current_cpu) { + qemu_cpu_kick(current_cpu); + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); + } +} diff --git a/stubs/icount.c b/stubs/icount.c index f13c43568b..6df8c2bf7d 100644 --- a/stubs/icount.c +++ b/stubs/icount.c @@ -43,3 +43,7 @@ void icount_account_warp_timer(void) { abort(); } + +void icount_notify_exit(void) +{ +} diff --git a/util/async.c b/util/async.c index 554ba70cca..75f50f47c4 100644 --- a/util/async.c +++ b/util/async.c @@ -33,6 +33,7 @@ #include "block/raw-aio.h" #include "qemu/coroutine_int.h" #include "qemu/coroutine-tls.h" +#include "sysemu/cpu-timers.h" #include "trace.h" /***********************************************************/ @@ -185,6 +186,13 @@ void qemu_bh_schedule_idle(QEMUBH *bh) void qemu_bh_schedule(QEMUBH *bh) { aio_bh_enqueue(bh, BH_SCHEDULED); + /* + * Workaround for record/replay. + * vCPU execution should be suspended when new BH is set. + * This is needed to avoid guest timeouts caused + * by the long cycles of the execution. + */ + icount_notify_exit(); } /* This func is async.