From patchwork Fri Jun 28 18:26:22 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 255652 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 733962C008F for ; Sat, 29 Jun 2013 05:41:48 +1000 (EST) Received: from localhost ([::1]:57395 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsdRF-0001lf-4Q for incoming@patchwork.ozlabs.org; Fri, 28 Jun 2013 14:30:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60500) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsdNv-0005Ra-Rf for qemu-devel@nongnu.org; Fri, 28 Jun 2013 14:27:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UsdNu-0000Iq-3w for qemu-devel@nongnu.org; Fri, 28 Jun 2013 14:27:03 -0400 Received: from mail-ee0-x236.google.com ([2a00:1450:4013:c00::236]:48595) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsdNt-0000Ig-IF for qemu-devel@nongnu.org; Fri, 28 Jun 2013 14:27:01 -0400 Received: by mail-ee0-f54.google.com with SMTP id t10so1192440eei.27 for ; Fri, 28 Jun 2013 11:27:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:subject:date:message-id:x-mailer:in-reply-to :references; bh=e6rHha0rYKydQmmNbclfVGqhII51w1V4F9tjdXRnUdc=; b=u7/rzoKEIHzBFg9oOPRpYeWwggIihEucAv4uzyTxom2Z/6G9W/uN9yrWLWJQ1I6GAK RSEmwUC6HxJhdOvHZqJvTVP3uQH/2Ty9eaWXmaVH7i4JGGruuFed8tIUEQl4Lowi26Nh sg30jazfAEibMIZ7KisFc8oJhNgs5MEnq8cx9RPWaaZuyj0AHONumGEXBOyYaPg07gSY XSaCUliA06bXHmXNJ254r0Sf8J91fGnICitUanGljUpRWLnH6XkQGyWwqLhtRXaxnBXa znGQ7v8xy9HrIMi//i6k7XrPrwR95FV5kk8XlWUgc3mATQQK02OzQGwNprEMqNl5pBEH uLmA== X-Received: by 10.14.203.194 with SMTP id f42mr14992477eeo.53.1372444020826; Fri, 28 Jun 2013 11:27:00 -0700 (PDT) Received: from playground.lan (net-37-116-217-184.cust.dsl.vodafone.it. [37.116.217.184]) by mx.google.com with ESMTPSA id o5sm12035344eef.5.2013.06.28.11.26.58 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 28 Jun 2013 11:26:59 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 28 Jun 2013 20:26:22 +0200 Message-Id: <1372444009-11544-4-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.1.4 In-Reply-To: <1372444009-11544-1-git-send-email-pbonzini@redhat.com> References: <1372444009-11544-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4013:c00::236 Subject: [Qemu-devel] [PATCH 03/30] memory: add reference counting to FlatView X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org With this change, a FlatView can be used even after a concurrent update has replaced it. Because we do not have RCU, we use a mutex to protect the small critical sections that read/write the as->current_map pointer. Accesses to the FlatView can be done outside the mutex. If a MemoryRegion will be used after the FlatView is unref-ed (or after a MemoryListener callback is returned), a reference has to be added to that MemoryRegion. For example, memory_region_find adds a reference to the MemoryRegion that it returns. Signed-off-by: Paolo Bonzini --- memory.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/memory.c b/memory.c index 319894e..bb92e17 100644 --- a/memory.c +++ b/memory.c @@ -29,12 +29,26 @@ static unsigned memory_region_transaction_depth; static bool memory_region_update_pending; static bool global_dirty_log = false; +/* flat_view_mutex is taken around reading as->current_map; the critical + * section is extremely short, so I'm using a single mutex for every AS. + * We could also RCU for the read-side. + * + * The BQL is taken around transaction commits, hence both locks are taken + * while writing to as->current_map (with the BQL taken outside). + */ +static QemuMutex flat_view_mutex; + static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners = QTAILQ_HEAD_INITIALIZER(memory_listeners); static QTAILQ_HEAD(, AddressSpace) address_spaces = QTAILQ_HEAD_INITIALIZER(address_spaces); +static void memory_init(void) +{ + qemu_mutex_init(&flat_view_mutex); +} + typedef struct AddrRange AddrRange; /* @@ -225,6 +239,7 @@ struct FlatRange { * order. */ struct FlatView { + unsigned ref; FlatRange *ranges; unsigned nr; unsigned nr_allocated; @@ -246,6 +261,7 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b) static void flatview_init(FlatView *view) { + view->ref = 1; view->ranges = NULL; view->nr = 0; view->nr_allocated = 0; @@ -279,6 +295,18 @@ static void flatview_destroy(FlatView *view) g_free(view); } +static void flatview_ref(FlatView *view) +{ + __sync_fetch_and_add(&view->ref, 1); +} + +static void flatview_unref(FlatView *view) +{ + if (__sync_fetch_and_sub(&view->ref, 1) == 1) { + flatview_destroy(view); + } +} + static bool can_merge(FlatRange *r1, FlatRange *r2) { return int128_eq(addrrange_end(r1->addr), r2->addr.start) @@ -578,6 +606,17 @@ static void address_space_add_del_ioeventfds(AddressSpace *as, } } +static FlatView *address_space_get_flatview(AddressSpace *as) +{ + FlatView *view; + + qemu_mutex_lock(&flat_view_mutex); + view = as->current_map; + flatview_ref(view); + qemu_mutex_unlock(&flat_view_mutex); + return view; +} + static void address_space_update_ioeventfds(AddressSpace *as) { FlatView *view; @@ -587,7 +626,7 @@ static void address_space_update_ioeventfds(AddressSpace *as) AddrRange tmp; unsigned i; - view = as->current_map; + view = address_space_get_flatview(as); FOR_EACH_FLAT_RANGE(fr, view) { for (i = 0; i < fr->mr->ioeventfd_nb; ++i) { tmp = addrrange_shift(fr->mr->ioeventfds[i].addr, @@ -609,6 +648,7 @@ static void address_space_update_ioeventfds(AddressSpace *as) g_free(as->ioeventfds); as->ioeventfds = ioeventfds; as->ioeventfd_nb = ioeventfd_nb; + flatview_unref(view); } static void address_space_update_topology_pass(AddressSpace *as, @@ -676,14 +716,25 @@ static void address_space_update_topology_pass(AddressSpace *as, static void address_space_update_topology(AddressSpace *as) { - FlatView *old_view = as->current_map; + FlatView *old_view = address_space_get_flatview(as); FlatView *new_view = generate_memory_topology(as->root); address_space_update_topology_pass(as, old_view, new_view, false); address_space_update_topology_pass(as, old_view, new_view, true); + qemu_mutex_lock(&flat_view_mutex); + flatview_unref(as->current_map); as->current_map = new_view; - flatview_destroy(old_view); + qemu_mutex_unlock(&flat_view_mutex); + + /* Note that all the old MemoryRegions are still alive up to this + * point. This relieves most MemoryListeners from the need to + * ref/unref the MemoryRegions they get---unless they use them + * outside the iothread mutex, in which case precise reference + * counting is necessary. + */ + flatview_unref(old_view); + address_space_update_ioeventfds(as); } @@ -1146,12 +1197,13 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr) FlatRange *fr; QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { - FlatView *view = as->current_map; + FlatView *view = address_space_get_flatview(as); FOR_EACH_FLAT_RANGE(fr, view) { if (fr->mr == mr) { MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync); } } + flatview_unref(view); } } @@ -1203,7 +1255,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa AddrRange tmp; MemoryRegionSection section; - view = as->current_map; + view = address_space_get_flatview(as); FOR_EACH_FLAT_RANGE(fr, view) { if (fr->mr == mr) { section = (MemoryRegionSection) { @@ -1229,6 +1281,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa } } } + flatview_unref(view); } static void memory_region_update_coalesced_range(MemoryRegion *mr) @@ -1520,7 +1573,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, as = memory_region_to_address_space(root); range = addrrange_make(int128_make64(addr), int128_make64(size)); - view = as->current_map; + view = address_space_get_flatview(as); fr = flatview_lookup(view, range); if (!fr) { return ret; @@ -1541,6 +1594,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, ret.readonly = fr->readonly; memory_region_ref(ret.mr); + flatview_unref(view); return ret; } @@ -1549,10 +1603,11 @@ void address_space_sync_dirty_bitmap(AddressSpace *as) FlatView *view; FlatRange *fr; - view = as->current_map; + view = address_space_get_flatview(as); FOR_EACH_FLAT_RANGE(fr, view) { MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync); } + flatview_unref(view); } void memory_global_dirty_log_start(void) @@ -1584,7 +1639,7 @@ static void listener_add_address_space(MemoryListener *listener, } } - view = as->current_map; + view = address_space_get_flatview(as); FOR_EACH_FLAT_RANGE(fr, view) { MemoryRegionSection section = { .mr = fr->mr, @@ -1598,6 +1653,7 @@ static void listener_add_address_space(MemoryListener *listener, listener->region_add(listener, §ion); } } + flatview_unref(view); } void memory_listener_register(MemoryListener *listener, AddressSpace *filter) @@ -1631,6 +1687,10 @@ void memory_listener_unregister(MemoryListener *listener) void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name) { + if (QTAILQ_EMPTY(&address_spaces)) { + memory_init(); + } + memory_region_transaction_begin(); as->root = root; as->current_map = g_new(FlatView, 1); @@ -1652,9 +1712,8 @@ void address_space_destroy(AddressSpace *as) memory_region_transaction_commit(); QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); address_space_destroy_dispatch(as); - flatview_destroy(as->current_map); + flatview_unref(as->current_map); g_free(as->name); - g_free(as->current_map); g_free(as->ioeventfds); }