[{"id":1773166,"web_url":"http://patchwork.ozlabs.org/comment/1773166/","msgid":"","list_archive_url":null,"date":"2017-09-21T23:06:12","subject":"Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of\n\taddress_space_init_shareable","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 21/09/2017 10:51, Alexey Kardashevskiy wrote:\n> Since FlatViews are shared now and ASes not, this gets rid of\n> address_space_init_shareable().\n> \n> This should cause no behavioural change.\n> \n> Signed-off-by: Alexey Kardashevskiy \n> ---\n> Changes:\n> v3:\n> * now removes @malloced and @ref_count, used to be in the previos patch\n> ---\n> include/exec/memory.h | 19 -------------------\n> include/hw/arm/armv7m.h | 2 +-\n> cpus.c | 5 +++--\n> hw/arm/armv7m.c | 9 ++++-----\n> memory.c | 22 +---------------------\n> target/arm/cpu.c | 16 ++++++++--------\n> target/i386/cpu.c | 5 +++--\n> 7 files changed, 20 insertions(+), 58 deletions(-)\n> \n> diff --git a/include/exec/memory.h b/include/exec/memory.h\n> index 2f4f56cf40..402824c6f2 100644\n> --- a/include/exec/memory.h\n> +++ b/include/exec/memory.h\n> @@ -309,8 +309,6 @@ struct AddressSpace {\n> struct rcu_head rcu;\n> char *name;\n> MemoryRegion *root;\n> - int ref_count;\n> - bool malloced;\n> \n> /* Accessed via RCU. */\n> struct FlatView *current_map;\n> @@ -1586,23 +1584,6 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,\n> void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);\n> \n> /**\n> - * address_space_init_shareable: return an address space for a memory region,\n> - * creating it if it does not already exist\n> - *\n> - * @root: a #MemoryRegion that routes addresses for the address space\n> - * @name: an address space name. The name is only used for debugging\n> - * output.\n> - *\n> - * This function will return a pointer to an existing AddressSpace\n> - * which was initialized with the specified MemoryRegion, or it will\n> - * create and initialize one if it does not already exist. The ASes\n> - * are reference-counted, so the memory will be freed automatically\n> - * when the AddressSpace is destroyed via address_space_destroy.\n> - */\n> -AddressSpace *address_space_init_shareable(MemoryRegion *root,\n> - const char *name);\n> -\n> -/**\n> * address_space_destroy: destroy an address space\n> *\n> * Releases all resources associated with an address space. After an address space\n> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h\n> index 10eb058027..008000fe11 100644\n> --- a/include/hw/arm/armv7m.h\n> +++ b/include/hw/arm/armv7m.h\n> @@ -21,7 +21,7 @@ typedef struct {\n> SysBusDevice parent_obj;\n> /*< public >*/\n> \n> - AddressSpace *source_as;\n> + AddressSpace source_as;\n> MemoryRegion iomem;\n> uint32_t base;\n> MemoryRegion *source_memory;\n> diff --git a/cpus.c b/cpus.c\n> index 9bed61eefc..c9a624003a 100644\n> --- a/cpus.c\n> +++ b/cpus.c\n> @@ -1764,8 +1764,9 @@ void qemu_init_vcpu(CPUState *cpu)\n> /* If the target cpu hasn't set up any address spaces itself,\n> * give it the default one.\n> */\n> - AddressSpace *as = address_space_init_shareable(cpu->memory,\n> - \"cpu-memory\");\n> + AddressSpace *as = g_new0(AddressSpace, 1);\n> +\n> + address_space_init(as, cpu->memory, \"cpu-memory\");\n> cpu->num_ases = 1;\n> cpu_address_space_init(cpu, as, 0);\n> }\n> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c\n> index b64a409b40..4900339646 100644\n> --- a/hw/arm/armv7m.c\n> +++ b/hw/arm/armv7m.c\n> @@ -41,7 +41,7 @@ static MemTxResult bitband_read(void *opaque, hwaddr offset,\n> \n> /* Find address in underlying memory and round down to multiple of size */\n> addr = bitband_addr(s, offset) & (-size);\n> - res = address_space_read(s->source_as, addr, attrs, buf, size);\n> + res = address_space_read(&s->source_as, addr, attrs, buf, size);\n> if (res) {\n> return res;\n> }\n> @@ -66,7 +66,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,\n> \n> /* Find address in underlying memory and round down to multiple of size */\n> addr = bitband_addr(s, offset) & (-size);\n> - res = address_space_read(s->source_as, addr, attrs, buf, size);\n> + res = address_space_read(&s->source_as, addr, attrs, buf, size);\n> if (res) {\n> return res;\n> }\n> @@ -79,7 +79,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,\n> } else {\n> buf[bitpos >> 3] &= ~bit;\n> }\n> - return address_space_write(s->source_as, addr, attrs, buf, size);\n> + return address_space_write(&s->source_as, addr, attrs, buf, size);\n> }\n> \n> static const MemoryRegionOps bitband_ops = {\n> @@ -111,8 +111,7 @@ static void bitband_realize(DeviceState *dev, Error **errp)\n> return;\n> }\n> \n> - s->source_as = address_space_init_shareable(s->source_memory,\n> - \"bitband-source\");\n> + address_space_init(&s->source_as, s->source_memory, \"bitband-source\");\n> }\n> \n> /* Board init. */\n> diff --git a/memory.c b/memory.c\n> index 4c28b91890..57e47e990f 100644\n> --- a/memory.c\n> +++ b/memory.c\n> @@ -2739,9 +2739,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)\n> {\n> memory_region_ref(root);\n> memory_region_transaction_begin();\n> - as->ref_count = 1;\n> as->root = root;\n> - as->malloced = false;\n> as->current_map = NULL;\n> as->ioeventfd_nb = 0;\n> as->ioeventfds = NULL;\n> @@ -2754,37 +2752,19 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)\n> \n> static void do_address_space_destroy(AddressSpace *as)\n> {\n> - bool do_free = as->malloced;\n> -\n> assert(QTAILQ_EMPTY(&as->listeners));\n> \n> flatview_unref(as->current_map);\n> g_free(as->name);\n> g_free(as->ioeventfds);\n> memory_region_unref(as->root);\n> - if (do_free) {\n> - g_free(as);\n> - }\n> -}\n> -\n> -AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)\n> -{\n> - AddressSpace *as;\n> -\n> - as = g_malloc0(sizeof *as);\n> - address_space_init(as, root, name);\n> - as->malloced = true;\n> - return as;\n> + g_free(as);\n\nThis g_free is wrong; /i386/virtio/blk/pci/hotplug in\ntests/virtio-blk-test is a pretty reliable reproducer.\n\nPaolo\n\n> }\n> \n> void address_space_destroy(AddressSpace *as)\n> {\n> MemoryRegion *root = as->root;\n> \n> - as->ref_count--;\n> - if (as->ref_count) {\n> - return;\n> - }\n> /* Flush out anything from MemoryListeners listening in on this */\n> memory_region_transaction_begin();\n> as->root = NULL;\n> diff --git a/target/arm/cpu.c b/target/arm/cpu.c\n> index 412e94c7ad..bba24f4590 100644\n> --- a/target/arm/cpu.c\n> +++ b/target/arm/cpu.c\n> @@ -684,6 +684,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)\n> CPUARMState *env = &cpu->env;\n> int pagebits;\n> Error *local_err = NULL;\n> +#ifndef CONFIG_USER_ONLY\n> + AddressSpace *as;\n> +#endif\n> \n> cpu_exec_realizefn(cs, &local_err);\n> if (local_err != NULL) {\n> @@ -874,24 +877,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)\n> \n> #ifndef CONFIG_USER_ONLY\n> if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {\n> - AddressSpace *as;\n> + as = g_new0(AddressSpace, 1);\n> \n> cs->num_ases = 2;\n> \n> if (!cpu->secure_memory) {\n> cpu->secure_memory = cs->memory;\n> }\n> - as = address_space_init_shareable(cpu->secure_memory,\n> - \"cpu-secure-memory\");\n> + address_space_init(as, cpu->secure_memory, \"cpu-secure-memory\");\n> cpu_address_space_init(cs, as, ARMASIdx_S);\n> } else {\n> cs->num_ases = 1;\n> }\n> -\n> - cpu_address_space_init(cs,\n> - address_space_init_shareable(cs->memory,\n> - \"cpu-memory\"),\n> - ARMASIdx_NS);\n> + as = g_new0(AddressSpace, 1);\n> + address_space_init(as, cs->memory, \"cpu-memory\");\n> + cpu_address_space_init(cs, as, ARMASIdx_NS);\n> #endif\n> \n> qemu_init_vcpu(cs);\n> diff --git a/target/i386/cpu.c b/target/i386/cpu.c\n> index 4b0fa0613b..b0b123a571 100644\n> --- a/target/i386/cpu.c\n> +++ b/target/i386/cpu.c\n> @@ -3741,10 +3741,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)\n> \n> #ifndef CONFIG_USER_ONLY\n> if (tcg_enabled()) {\n> - AddressSpace *as_normal = address_space_init_shareable(cs->memory,\n> - \"cpu-memory\");\n> + AddressSpace *as_normal = g_new0(AddressSpace, 1);\n> AddressSpace *as_smm = g_new(AddressSpace, 1);\n> \n> + address_space_init(as_normal, cs->memory, \"cpu-memory\");\n> +\n> cpu->cpu_as_mem = g_new(MemoryRegion, 1);\n> cpu->cpu_as_root = g_new(MemoryRegion, 1);\n> \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xysgg62PXz9t2S\n\tfor ;\n\tFri, 22 Sep 2017 09:06:51 +1000 (AEST)","from localhost ([::1]:55812 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dvAYS-0004Ln-74\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 19:06:48 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33318)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dvAY3-0004LK-HB\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 19:06:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dvAY0-0007Uj-9C\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 19:06:23 -0400","from mail-wm0-f48.google.com ([74.125.82.48]:44243)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from ) id 1dvAXz-0007Ni-Vq\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 19:06:20 -0400","by mail-wm0-f48.google.com with SMTP id m127so2688204wmm.1\n\tfor ; Thu, 21 Sep 2017 16:06:17 -0700 (PDT)","from [192.168.10.165]\n\t(dynamic-adsl-78-12-246-117.clienti.tiscali.it.\n\t[78.12.246.117]) by smtp.gmail.com with ESMTPSA id\n\tb58sm3194561wrg.86.2017.09.21.16.06.13\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 21 Sep 2017 16:06:14 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=Jcb7GD8wQAONQlj3gFf7c3UouKAcZyKyxFfCbjrLH2E=;\n\tb=lsDpJupdsK6c3N7bU33UVC1iprJB1QVtHbzEqy+CDCXgkL4i6x9unjsvfT9OQG0OuG\n\tEFB1RCePwpnIOFtpbqK9t0ayRHtAd/0s6PWU/tvKSIhkHOJw/oLqunAQLes1KbYF/k9f\n\tFJp4HDso4U8vsltfmRiQkJTxpRar1BqXap26hd8uAfpgzxPq126i3SZj3KbfnlmWRxtq\n\t/PtP9HLwBUlcu+pW27mIOOZQnsrbJG97Qc8nd7SbgXl0RDm85bjdi1cfw2N+ulxRZ/u0\n\tHmm76GNwfaf/cVvLH7f6HpKpY80nLDSxnCuo93XsMZqVp1/S88LTGBaTaUe0Pf4+TuH5\n\tlyvw==","X-Gm-Message-State":"AHPjjUiNUKycan5WNJaXbfrjbAtmTpVEN1klQuSSvMD6UOmh9kILtOBe\n\t/fRCy0KP7Ol/eDOfUMTRssveq9Olkq8=","X-Google-Smtp-Source":"AOwi7QAof6WOQqFAa83rWwb38WuIgiiDOCCUQ8Cd59wzaVVCf6MIWHSUGTRxJz7v55hRTiFyGdVJmA==","X-Received":"by 10.28.58.136 with SMTP id h130mr2114828wma.56.1506035175790; \n\tThu, 21 Sep 2017 16:06:15 -0700 (PDT)","To":"Alexey Kardashevskiy , qemu-devel@nongnu.org","References":"<20170921085110.25598-1-aik@ozlabs.ru>\n\t<20170921085110.25598-17-aik@ozlabs.ru>","From":"Paolo Bonzini ","Message-ID":"","Date":"Fri, 22 Sep 2017 01:06:12 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170921085110.25598-17-aik@ozlabs.ru>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"74.125.82.48","Subject":"Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of\n\taddress_space_init_shareable","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]