[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB |
Date: |
Wed, 15 Mar 2017 09:36:16 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 14/03/2017 22:23, Xu, Anthony wrote:
>> flatview_unref can call object_unref and thus reach:
>
> Okay, flatview_unref is the one you worried about,
>
> Flatview_unref is registered as a RCU callback only in
> address_space_update_topology,
> Strangely, it is registered as a RCU callback, and is called directly in this
> function.
> Basially flatview_unref is called twice for old view.
The first unref is done after as->current_map is overwritten.
as->current_map is accessed under RCU, so it needs call_rcu. It
balances the initial reference that is present since flatview_init.
The second unref is done to balance the flatview_ref in
address_space_get_flatview.
> There are some comments,
> but it is not clear to me, is this a bug or by design? Is flatview_destroy
> called in current thread
> or RCU thread?
You cannot know, because there are also other callers of
address_space_get_flatview. Usually it's from the RCU thread.
> Let me split the patch, Do you think below patch is correct?
>
> --- a/memory.c
> +++ b/memory.c
> @@ -1503,15 +1503,9 @@ static void memory_region_finalize(Object *obj)
> * and cause an infinite loop.
> */
> mr->enabled = false;
> - memory_region_transaction_begin();
> - while (!QTAILQ_EMPTY(&mr->subregions)) {
> - MemoryRegion *subregion = QTAILQ_FIRST(&mr->subregions);
> - memory_region_del_subregion(mr, subregion);
> - }
> - memory_region_transaction_commit();
> -
> + assert(QTAILQ_EMPTY(&mr->subregions));
> mr->destructor(mr);
> - memory_region_clear_coalescing(m
> + assert(QTAILQ_EMPTY(&mr->coalesced));
> g_free((char *)mr->name);
> g_free(mr->ioeventfds);
> }
No. Callers can:
- let memory_region_finalize remove subregions (commit 2e2b8eb, "memory:
allow destroying a non-empty MemoryRegion", 2015-10-09)
- let memory_region_finalize disable coalesced I/O (in fact there are no
callers of memory_region_clear_coalescing outside memory.c)
> Then let us take a look at flatview_unref, memory_region_unref may call any
> QOM finalize through
> Object_unref(mr->owner), that's your concern, I got it. It is way too
> complicated to look at each QOM
> object release callbacks and each QOM property release callbacks. I gave up
> this path.
> How about fall back to synchronize_rcu?
I'm afraid it would be too slow, but you can test.
Something like the kernel's synchronize_rcu_expedited would work, but we
don't have anything like that in QEMU yet.
> As for address space, the RCU read lock is used to protect PhysPageMap, but
> not the regular MemoryRegions,
Nope. The RCU read lock protects all MemoryRegions through
flatview_unref: RCU keeps the FlatView alive, and the FlatView keeps
MemoryRegions alive.
Hence...
> The MemoryRegions returned from address_space_translate are regular
> MemoryRegions, so
> address_space_write_continue and address_space_read_continue don't need RCU
> read lock.
... this is wrong too.
Paolo
- [Qemu-devel] [PATCH v1 1/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Yang Zhong, 2017/03/10
- [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Yang Zhong, 2017/03/10
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/10
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/10
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/11
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/14
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/14
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/14
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/15
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/15
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Zhong, Yang, 2017/03/15
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/16
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/22
- Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Xu, Anthony, 2017/03/22
Re: [Qemu-devel] [PATCH v1 1/2] reduce qemu's heap Rss size from 12252kB to 2752KB, Paolo Bonzini, 2017/03/10