| Message ID | 20220801164527.3134765-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-7.1] icount: Take iothread lock when running QEMU timers | expand |
On 8/1/22 09:45, Peter Maydell wrote: > The function icount_prepare_for_run() is called with the iothread > unlocked, but it can call icount_notify_aio_contexts() which will > run qemu timer handlers. Those are supposed to be run only with > the iothread lock held, so take the lock while we do that. > > Since icount mode runs everything on a single thread anyway, > not holding the lock is likely mostly not going to introduce > races, but it can cause us to trip over assertions that we > do hold the lock, such as the one reported in issue 1130. > > Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1130 > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > accel/tcg/tcg-accel-ops-icount.c | 6 ++++++ > 1 file changed, 6 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Tested-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> On 01.08.2022 19:45, Peter Maydell wrote: > The function icount_prepare_for_run() is called with the iothread > unlocked, but it can call icount_notify_aio_contexts() which will > run qemu timer handlers. Those are supposed to be run only with > the iothread lock held, so take the lock while we do that. > > Since icount mode runs everything on a single thread anyway, > not holding the lock is likely mostly not going to introduce > races, but it can cause us to trip over assertions that we > do hold the lock, such as the one reported in issue 1130. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1130 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > accel/tcg/tcg-accel-ops-icount.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c > index 8f1dda4344c..84cc7421be8 100644 > --- a/accel/tcg/tcg-accel-ops-icount.c > +++ b/accel/tcg/tcg-accel-ops-icount.c > @@ -109,7 +109,13 @@ void icount_prepare_for_run(CPUState *cpu) > replay_mutex_lock(); > > if (cpu->icount_budget == 0) { > + /* > + * We're called without the iothread lock, so must take it while > + * we're calling timer handlers. > + */ > + qemu_mutex_lock_iothread(); > icount_notify_aio_contexts(); > + qemu_mutex_unlock_iothread(); > } > } >
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c index 8f1dda4344c..84cc7421be8 100644 --- a/accel/tcg/tcg-accel-ops-icount.c +++ b/accel/tcg/tcg-accel-ops-icount.c @@ -109,7 +109,13 @@ void icount_prepare_for_run(CPUState *cpu) replay_mutex_lock(); if (cpu->icount_budget == 0) { + /* + * We're called without the iothread lock, so must take it while + * we're calling timer handlers. + */ + qemu_mutex_lock_iothread(); icount_notify_aio_contexts(); + qemu_mutex_unlock_iothread(); } }
The function icount_prepare_for_run() is called with the iothread unlocked, but it can call icount_notify_aio_contexts() which will run qemu timer handlers. Those are supposed to be run only with the iothread lock held, so take the lock while we do that. Since icount mode runs everything on a single thread anyway, not holding the lock is likely mostly not going to introduce races, but it can cause us to trip over assertions that we do hold the lock, such as the one reported in issue 1130. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1130 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- accel/tcg/tcg-accel-ops-icount.c | 6 ++++++ 1 file changed, 6 insertions(+)