On Mon, Aug 5, 2013 at 6:00 PM, Alex Bligh <a...@alex.org.uk> wrote: > Pingfan, > > > --On 5 August 2013 15:33:22 +0800 Liu Ping Fan <pingf...@linux.vnet.ibm.com> > wrote: > >> The patches has been rebased onto Alex's [RFC] [PATCHv5 00/16] aio / >> timers: Add AioContext timers and use ppoll >> permalink.gmane.org/gmane.comp.emulators.qemu/226333 >> >> For some other complied error issue, I can not finish compiling, will fix >> it later. >> >> Changes since last version: >> 1. drop the overlap partition and leave only thread-safe stuff >> 2. For timers_state, since currently, only vm_clock can be read outside >> BQL. There is no protection with ticks(since the protection will cost >> more in read_tsc path). 3. use light weight QemuEvent to re-implement >> the qemu_clock_enable(foo,false) > > > I think you may need to protect a little more. > Yes. There is still race issue left. If Stefanha and you will not do it, I am pleased to do that.
> For instance, do we need to take a lock whilst traversing a QEMUTimerList? > I hope the answer to this is no. The design idea of my stuff was that only > one thread would be manipulating or traversing this list. As the notify > function is a property of the QEMUTimerList itself, no traversal is > necessary for that. However, the icount stuff (per my patch 15) does > look at the deadlines for the vm_clock QEMUTimerLists (which is the > first entry). Is that always going to be thread safe? Before the icount > stuff, I was pretty certain this did not need a lock, but perhaps > it does now. If the soonest deadline on any QEMUTimerList was stored > in a 64 bit atomic variable, this might remove the need for a lock; > it's possible that putting some memory barrier operations in might > be enough. > > Also, we maintain a per-clock list of QEMUTimerLists. This list is > traversed by the icount stuff, by things adding and removing timers > (e.g. creation/deletion of AioContext) and whenever a QEMUClock is > reenabled. I think this DOES need a lock. Aside from the icount > stuff, usage is very infrequent. It's far more frequent with the > icount stuff, so it should probably be optimised for that case. > > -- > Alex Bligh >
