Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL

[Robert Foley]

On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 05/08/20 21:18, Richard Henderson wrote:
> > On 8/5/20 11:12 AM, Robert Foley wrote:
> >> This change removes the implied BQL from the cpu_handle_interrupt,
> >> and cpu_handle_exception paths. This BQL acquire is being pushed
> >> down into the per arch implementation.
> >>
> >> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> >> ---
> >>  accel/tcg/cpu-exec.c | 19 +++++++++++--------
> >>  1 file changed, 11 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> >> index 80d0e649b2..8e2bfd97a1 100644
> >> --- a/accel/tcg/cpu-exec.c
> >> +++ b/accel/tcg/cpu-exec.c
> >> @@ -517,9 +517,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, 
> >> int *ret)
> >>  #else
> >>          if (replay_exception()) {
> >>              CPUClass *cc = CPU_GET_CLASS(cpu);
> >> -            qemu_mutex_lock_iothread();
> >>              cc->do_interrupt(cpu);
> >> -            qemu_mutex_unlock_iothread();
> >>              cpu->exception_index = -1;
> >>
> >
> > This patch is not bisectable.  The removal of the lock here needs to happen 
> > at
> > the end, or something.
>
> Indeed the series should be structured like this:
>
> 1) rename all *_do_interrupt functions to *_do_interrupt_locked
>
> 2) add back *_do_interrupt that takes the BQL and calls
> *_do_interrupt_locked, point ->do_interrupt to it, remove the BQL from
> cpu-exec.c
>
> 3) modify the cpu_mutex and BQL critical sections around
> ->cpu_exec_interrupt, so that the BQL critical section covers just the
> call to ->cpu_exec_interrupt.  Document which fields are now covered by
> cpu_mutex.
>
> 4/5) same as 1/2 for ->cpu_exec_interrupt
>
> Patches 1/2 would be pretty large, but they're trivial to review just by
> grepping for "->do_interrupt\s*=", and likewise for 4/5.
>

Thanks for the details !

It seems like we will have 3 separate patches for this series, 1/2, 3, and 4/5.

We will go in this direction.

Thanks,
-Rob

> Thanks,
>
> Paolo
>