Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem

[Avi Kivity]

On 08/27/2012 12:17 PM, Anthony Liguori wrote:
> Avi Kivity <address@hidden> writes:
>
> > On 08/27/2012 09:24 AM, Anthony Liguori wrote:
> >> >
> >> > I'm sure we should leave existing code alone wherever possible, focusing
> >> > on providing alternative versions for those paths that matter. Example:
> >> > Most timers are fine under BQL. But some sensitive devices (RTC or HPET
> >> > as clock source) will want their own timers. So the approach is to
> >> > instantiate a separate, also prioritizeable instance of the timer
> >> > subsystem for them and be done.
> >>
> >> I disagree.  I think we conver the timer subsystem to be lockless and
> >> then let some devices acquire the BQL during dispatch.
> >
> > I agree with your disagreement but disagree with the rest.  The timer
> > subsystem should have its own internal locking that callers will not be
> > aware of.  Requiring devices to acquire the bql will lead to
> > deadlocks.
>
> Err, I completely mispoke there.
>
> I meant, to say, we should convert the timer subsystem to be re-entrant
> and then let some devices acquire the BQL during dispatch.

That would be all devices, with converted devices not acquiring it as
they are converted.

> Existing users of the time API should get the BQL acquired automagically
> for them.  The new API would not hold the BQL during dispatch but the
> old API, implemented in terms of the new API, would acquire it on behalf
> of the caller.
>
> As a rule, if a device relies on the BQL, it must use the old APIs.  If
> a device uses the new APIs, it must not acquire the BQL.

This exactly mirrors the plan with memory region conversion.

> > Note that fine-grained locking timers will also require reference
> > counting: you want to call the timer expiration callback after releasing
> > the timer subsystem lock, so you need to make sure the callback does not
> > go away.
>
> glib simply uses a single main loop lock to deal with all of this.  I
> think that's absolutely the right model.
>
> It's best to start this conversion using very coarse locking.  There's
> no need to start with ultra fine grain locking.

How does it work?  You have to drop this main loop lock to dispatch the
callback, so the data structure you use for dispatch is no longer protected.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.