Il 04/09/2013 22:48, Mike Day ha scritto: > > On Wed, Sep 4, 2013 at 3:58 PM, Paolo Bonzini <pbonz...@redhat.com > <mailto:pbonz...@redhat.com>> wrote: >> >> > @@ -1323,23 +1325,21 @@ static RAMBlock > *qemu_get_ram_block(ram_addr_t addr) >> > { >> > RAMBlock *block; >> > >> > - /* The list is protected by the iothread lock here. */ >> > + /* This assumes the iothread lock is taken here too. */ >> > + >> > block = ram_list.mru_block; >> > if (block && addr - block->offset < block->length) { >> > - goto found; >> > + return block; >> > } >> > - QTAILQ_FOREACH(block, &ram_list.blocks, next) { >> > + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { >> > if (addr - block->offset < block->length) { >> > - goto found; >> > + atomic_rcu_set(&ram_list.mru_block, block); >> >> I think this is not needed, block was already published into the block >> list. What is important is to order mru_block == NULL so that it >> happens before the node is removed. Which we don't do, but we can fix >> later. > > I am thinking of writing some macros and possibly reorganizing the ram > globals into a struct so that we can update it by exchanging pointers > atomically. It seems to disjoint right and error-prone now that we are > making it rcu-enabled.
Note that mru_block is set in hottish paths, so changes to mru_block need not result in copying the RAM globals. Only changes to the list would. But I'm not sure yet that copying the RAM globals struct is useful. All we need the version flag for, is to know whether a pointer from a previous RCU read-side critical section is still valid after a new rcu_read_lock(). Paolo
