[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new w
From: |
Alex Bligh |
Subject: |
Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper |
Date: |
Fri, 9 Aug 2013 23:57:50 +0100 |
On 9 Aug 2013, at 15:36, Paolo Bonzini wrote:
>>>
>>> Since we're doing a new API, I would prefer to have it as timer_init and
>>> aio_timer_init. We can remove the allocation completely, it is a
>>> useless indirection and we misuse it since we hardly ever call
>>> qemu_free_timer.
>>
>> Would that not require change the huge number of qemu_timer_new references
>> to use this new API? That sounds less than automatic! Not in favour of
>> that one.
>
> qemu_timer_new can remain for now (only waiting for the next
> mass-rewriting script to be written). I would just prefer to have the
> new AioContext-/TimerList-aware not do any allocation.
Though I've nearly completed this, I am only now (always the way)
having second thoughts about whether this is a good idea.
There are a large number of users of qemu_free_timer (now timer_free).
If someone does not call qemu_free_timer having called qemu_new_timer,
the timer sits there and basically does nothing.
If we go to the timer_init model, the timer will either be on the
stack or (more likely) inside some other struct on the heap, which
will likely have been freed. This means walking the timer list will
be dangerous.
This seems to add a good deal of fragility.
I've no objection to adding timer_init (especially as I've already
done it), but I wonder whether we're right to make this the only
interface for aio_timer etc. - i.e. maybe we should have aio_timer_new
and aio_timer_init.
WDYT?
--
Alex Bligh
- Re: [Qemu-devel] [RFC] [PATCHv8 12/30] aio / timers: aio_ctx_prepare sets timeout from AioContext timers, (continued)
[Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Alex Bligh, 2013/08/08
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Paolo Bonzini, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Alex Bligh, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Paolo Bonzini, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Alex Bligh, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Paolo Bonzini, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Alex Bligh, 2013/08/09
- Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Paolo Bonzini, 2013/08/09
Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper,
Alex Bligh <=
Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper, Paolo Bonzini, 2013/08/10
[Qemu-devel] [RFC] [PATCHv8 08/30] aio / timers: Untangle include files, Alex Bligh, 2013/08/08
[Qemu-devel] [RFC] [PATCHv8 14/30] aio / timers: Convert aio_poll to use AioContext timers' deadline, Alex Bligh, 2013/08/08
[Qemu-devel] [RFC] [PATCHv8 07/30] aio / timers: Split QEMUClock into QEMUClock and QEMUTimerList, Alex Bligh, 2013/08/08
[Qemu-devel] [RFC] [PATCHv8 15/30] aio / timers: Convert mainloop to use timeout, Alex Bligh, 2013/08/08
[Qemu-devel] [RFC] [PATCHv8 16/30] aio / timers: On timer modification, qemu_notify or aio_notify, Alex Bligh, 2013/08/08