On 09/19/2012 12:51 PM, Paolo Bonzini wrote:
> Il 19/09/2012 11:21, Avi Kivity ha scritto:
>>> > I don't know if the front-end (device) lock should come before or after
>>> > the back-end (e.g. netdev) lock in the hierarchy, but that's another
>>> > story.
>> I would say device -> backend. It's natural if the backend is the timer
>> subsystem, so extend it from there to the block and network layers. Of
>> course callbacks want it to be the other way round.
>
> Yes, that's what I wasn't sure about. In many cases I believe callbacks
> can just release the backend lock. It works for timers, for example:
>
> for(;;) {
> ts = clock->active_timers;
> if (!qemu_timer_expired_ns(ts, current_time)) {
> break;
> }
> /* remove timer from the list before calling the callback */
> clock->active_timers = ts->next;
> ts->next = NULL;
>
> /* run the callback (the timer list can be modified) */
> - ts->cb(ts->opaque);
> + cb = ts->cb;
> + opaque = ts->opaque;
> + unlock();
> + cb(opaque);
> + lock();
> }
>
> (The hunch is that ts could be deleted exactly at the moment the
> callback is unlocked. This can be solved with ref/unref on the opaque
> value, as you mention below).
Are you saying that this works as is or not? It does seem broken wrt
deletion; after qemu_del_timer() completes the caller expects the
callback not to be called.
This isn't trivial to guarantee, we need something like
dispatch_timer():
pending += 1
timer.ref()
drop lock
timer.cb()
take lock
timer.unref()
pending -= 1
notify
del_timer():
take lock
timer.unlink()
while pending:
wait for notification
drop lock
but, if del_timer is called with the device lock held, we deadlock. ugh.
--
error compiling committee.c: too many arguments to function
