Re: [Qemu-devel] [PATCH] qom/cpu: Remove cpu->exit_request from reset state

[Edgar E. Iglesias]

On Wed, Mar 12, 2014 at 12:03:19AM +0000, Peter Maydell wrote:
> On 11 March 2014 23:58, Andreas Färber <address@hidden> wrote:
> > Am 20.02.2014 16:58, schrieb Peter Maydell:
> >> On 16 February 2014 02:07, Edgar E. Iglesias <address@hidden> wrote:
> >>> On Sat, Feb 15, 2014 at 03:42:56PM +0000, Peter Maydell wrote:
> >>>> On 13 February 2014 05:07,  <address@hidden> wrote:
> >>>>> From: "Edgar E. Iglesias" <address@hidden>
> >>>>>
> >>>>> cpu->exit_request is part of the execution environment and should
> >>>>> not be cleared when a CPU resets.
> >>>>>
> >>>>> Otherwise, we might deadlock QEMU if a CPU resets while there is
> >>>>> I/O going on.
> >>>>>
> >>>>> Signed-off-by: Edgar E. Iglesias <address@hidden>
> >>>>> ---
> >>>>>  qom/cpu.c | 1 -
> >>>>>  1 file changed, 1 deletion(-)
> >>>>>
> >>>>> diff --git a/qom/cpu.c b/qom/cpu.c
> >>>>> index 9d62479..40d82dd 100644
> >>>>> --- a/qom/cpu.c
> >>>>> +++ b/qom/cpu.c
> >>>>> @@ -195,7 +195,6 @@ static void cpu_common_reset(CPUState *cpu)
> >>>>>          log_cpu_state(cpu, cc->reset_dump_flags);
> >>>>>      }
> >>>>>
> >>>>> -    cpu->exit_request = 0;
> >>>>>      cpu->interrupt_request = 0;
> >>>>>      cpu->current_tb = NULL;
> >>>>>      cpu->halted = 0;
> >>>>
> >>>> This looks kind of odd to me. What's the situation you see where
> >>>> this matters -- is the CPU resetting itself, or is some other device
> >>>> in another thread triggering the CPU reset? TCG or KVM?
> >>>
> >>> Seeing this in TCG. The CPU gets signaled by the IO thread while the
> >>> CPU is resetting itself. If the CPU looses the race, it clears its
> >>> exit_request leaving the IO thread waiting for the global lock
> >>> potentially forever.
> >>>
> >>> The CPU actually exits generated code but goes right back in because
> >>> there is no exit_request pending.
> >>
> >> Yes, having looked at the code I agree with you, so:
> >>
> >> Reviewed-by: Peter Maydell <address@hidden>
> >
> > Thanks, applied to qom-cpu (with clarified commit message):
> > https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> I'd forgotten about this, but it's a bugfix for a hang, right?

Hi, yes it's a bugfix for a hang.

> Seems to me like we ought to put it into 2.0 -- were you

Makes sense to me

Thanks,
Edgar


> planning to submit it via qom-cpu for 2.0?
> 
> thanks
> -- PMM