From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEijF-00011w-Ez for qemu-devel@nongnu.org; Wed, 28 Aug 2013 12:36:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEij9-0005Ko-EP for qemu-devel@nongnu.org; Wed, 28 Aug 2013 12:36:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13809) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEij9-0005Kd-5X for qemu-devel@nongnu.org; Wed, 28 Aug 2013 12:36:15 -0400 Message-ID: <521E26B8.7090801@redhat.com> Date: Wed, 28 Aug 2013 18:35:04 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1377705768-21996-1-git-send-email-ncmike@ncultra.org> In-Reply-To: <1377705768-21996-1-git-send-email-ncmike@ncultra.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] convert ram_list to RCU DQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Day Cc: Paul Mckenney , Mathew Desnoyers , qemu-devel@nongnu.org, Anthony Liguori Il 28/08/2013 18:02, Mike Day ha scritto: > @@ -457,8 +459,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > MemoryRegion *mr; > ram_addr_t current_addr; > > + rcu_read_lock(); > if (!block) > - block = QTAILQ_FIRST(&ram_list.blocks); > + block = QLIST_FIRST_RCU(&ram_list.blocks); > > while (true) { > mr = block->mr; > @@ -469,9 +472,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > } > if (offset >= block->length) { > offset = 0; > - block = QTAILQ_NEXT(block, next); > + block = QLIST_NEXT_RCU(block, next); > if (!block) { > - block = QTAILQ_FIRST(&ram_list.blocks); > + block = QLIST_FIRST_RCU(&ram_list.blocks); > complete_round = true; > ram_bulk_stage = false; > } > @@ -526,6 +529,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > } > } > } > + rcu_read_unlock(); block lives across calls to ram_save_block, which is why the mutex was locked in the caller (ram_save_iterate) rather than here. For a first conversion, keeping the long RCU critical sections is fine. We don't use RCU enough yet to care about delaying other call_rcu callbacks. We can later check push the check for ram_list.version inside ram_save_block, which should let us make the critical section smaller. But that would be a bit tricky, so it's better to do it in a separate patch. > @@ -828,13 +829,18 @@ static inline void *host_from_stream_offset(QEMUFile *f, > qemu_get_buffer(f, (uint8_t *)id, len); > id[len] = 0; > > - QTAILQ_FOREACH(block, &ram_list.blocks, next) { > - if (!strncmp(id, block->idstr, sizeof(id))) > - return memory_region_get_ram_ptr(block->mr) + offset; > + rcu_read_lock(); > + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > + if (!strncmp(id, block->idstr, sizeof(id))) { > + ptr = memory_region_get_ram_ptr(block->mr) + offset; > + goto unlock_out; > + } > } > > fprintf(stderr, "Can't find block %s!\n", id); > - return NULL; > +unlock_out: > + rcu_read_unlock(); > + return ptr; > } Similarly, here the critical section includes the caller, and block is living across calls to host. Again, for now just put all of ram_load under a huge RCU critical section. Later we can use ram_list.version to refresh the list and make the critical sections smaller. Paolo