| Message ID | 34720dd344455d0abee575d399caedebcc099e5a.1296133797.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
On 01/27/2011 03:09 PM, Jan Kiszka wrote: > If there is any pending request that requires us to leave the inner loop > if main_loop, makes sure we do this as soon as possible by enforcing > non-blocking IO processing. > > At this change, move variable definitions out of the inner loop to > improve readability. > > Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > --- > vl.c | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 5fad700..2ebc55b 100644 > --- a/vl.c > +++ b/vl.c > @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown; > > static void main_loop(void) > { > + bool nonblocking = false; > +#ifdef CONFIG_PROFILER > + int64_t ti; > +#endif > int r; > > qemu_main_loop_start(); > > for (;;) { > do { > - bool nonblocking = false; > -#ifdef CONFIG_PROFILER > - int64_t ti; > -#endif > #ifndef CONFIG_IOTHREAD > nonblocking = cpu_exec_all(); > + if (!vm_can_run()) { > + nonblocking = true; > + } Doesn't this cause vmstop to spin? We'll never execute main_loop_wait(false) if I read the code correctly?
On 2011-01-31 10:52, Avi Kivity wrote: > On 01/27/2011 03:09 PM, Jan Kiszka wrote: >> If there is any pending request that requires us to leave the inner loop >> if main_loop, makes sure we do this as soon as possible by enforcing >> non-blocking IO processing. >> >> At this change, move variable definitions out of the inner loop to >> improve readability. >> >> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >> --- >> vl.c | 11 +++++++---- >> 1 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 5fad700..2ebc55b 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown; >> >> static void main_loop(void) >> { >> + bool nonblocking = false; >> +#ifdef CONFIG_PROFILER >> + int64_t ti; >> +#endif >> int r; >> >> qemu_main_loop_start(); >> >> for (;;) { >> do { >> - bool nonblocking = false; >> -#ifdef CONFIG_PROFILER >> - int64_t ti; >> -#endif >> #ifndef CONFIG_IOTHREAD >> nonblocking = cpu_exec_all(); >> + if (!vm_can_run()) { >> + nonblocking = true; >> + } > > Doesn't this cause vmstop to spin? We'll never execute > main_loop_wait(false) if I read the code correctly? > The code path is not changed, we just poll instead of wait in main_loop_wait. Also, I didn't get your error scenario yet. Even if we left the loop here, what magic would main_loop_wait do to vmstop processing? The stop request is handled outside the loop, that's why we should leave ASAP. Jan
On 01/31/2011 01:22 PM, Jan Kiszka wrote: > On 2011-01-31 10:52, Avi Kivity wrote: > > On 01/27/2011 03:09 PM, Jan Kiszka wrote: > >> If there is any pending request that requires us to leave the inner loop > >> if main_loop, makes sure we do this as soon as possible by enforcing > >> non-blocking IO processing. > >> > >> At this change, move variable definitions out of the inner loop to > >> improve readability. > >> > >> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > >> --- > >> vl.c | 11 +++++++---- > >> 1 files changed, 7 insertions(+), 4 deletions(-) > >> > >> diff --git a/vl.c b/vl.c > >> index 5fad700..2ebc55b 100644 > >> --- a/vl.c > >> +++ b/vl.c > >> @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown; > >> > >> static void main_loop(void) > >> { > >> + bool nonblocking = false; > >> +#ifdef CONFIG_PROFILER > >> + int64_t ti; > >> +#endif > >> int r; > >> > >> qemu_main_loop_start(); > >> > >> for (;;) { > >> do { > >> - bool nonblocking = false; > >> -#ifdef CONFIG_PROFILER > >> - int64_t ti; > >> -#endif > >> #ifndef CONFIG_IOTHREAD > >> nonblocking = cpu_exec_all(); > >> + if (!vm_can_run()) { > >> + nonblocking = true; > >> + } > > > > Doesn't this cause vmstop to spin? We'll never execute > > main_loop_wait(false) if I read the code correctly? > > > > The code path is not changed, we just poll instead of wait in > main_loop_wait. Where do we wait then? > Also, I didn't get your error scenario yet. Even if we left the loop > here, what magic would main_loop_wait do to vmstop processing? The stop > request is handled outside the loop, that's why we should leave ASAP. I'm just missing the alternate place where we sleep. If there's no such place, we spin.
On 2011-01-31 14:17, Avi Kivity wrote: > On 01/31/2011 01:22 PM, Jan Kiszka wrote: >> On 2011-01-31 10:52, Avi Kivity wrote: >>> On 01/27/2011 03:09 PM, Jan Kiszka wrote: >>>> If there is any pending request that requires us to leave the inner loop >>>> if main_loop, makes sure we do this as soon as possible by enforcing >>>> non-blocking IO processing. >>>> >>>> At this change, move variable definitions out of the inner loop to >>>> improve readability. >>>> >>>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >>>> --- >>>> vl.c | 11 +++++++---- >>>> 1 files changed, 7 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/vl.c b/vl.c >>>> index 5fad700..2ebc55b 100644 >>>> --- a/vl.c >>>> +++ b/vl.c >>>> @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown; >>>> >>>> static void main_loop(void) >>>> { >>>> + bool nonblocking = false; >>>> +#ifdef CONFIG_PROFILER >>>> + int64_t ti; >>>> +#endif >>>> int r; >>>> >>>> qemu_main_loop_start(); >>>> >>>> for (;;) { >>>> do { >>>> - bool nonblocking = false; >>>> -#ifdef CONFIG_PROFILER >>>> - int64_t ti; >>>> -#endif >>>> #ifndef CONFIG_IOTHREAD >>>> nonblocking = cpu_exec_all(); >>>> + if (!vm_can_run()) { >>>> + nonblocking = true; >>>> + } >>> >>> Doesn't this cause vmstop to spin? We'll never execute >>> main_loop_wait(false) if I read the code correctly? >>> >> >> The code path is not changed, we just poll instead of wait in >> main_loop_wait. > > Where do we wait then? > >> Also, I didn't get your error scenario yet. Even if we left the loop >> here, what magic would main_loop_wait do to vmstop processing? The stop >> request is handled outside the loop, that's why we should leave ASAP. > > I'm just missing the alternate place where we sleep. If there's no such > place, we spin. > Probably you are misled by the name of vm_can_run. It should better be renamed to requests_pending or something like that - iow, it is only true if some request is pending and not if just the vm is in stop mode. Jan
diff --git a/vl.c b/vl.c index 5fad700..2ebc55b 100644 --- a/vl.c +++ b/vl.c @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown; static void main_loop(void) { + bool nonblocking = false; +#ifdef CONFIG_PROFILER + int64_t ti; +#endif int r; qemu_main_loop_start(); for (;;) { do { - bool nonblocking = false; -#ifdef CONFIG_PROFILER - int64_t ti; -#endif #ifndef CONFIG_IOTHREAD nonblocking = cpu_exec_all(); + if (!vm_can_run()) { + nonblocking = true; + } #endif #ifdef CONFIG_PROFILER ti = profile_getclock();
If there is any pending request that requires us to leave the inner loop if main_loop, makes sure we do this as soon as possible by enforcing non-blocking IO processing. At this change, move variable definitions out of the inner loop to improve readability. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- vl.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)