On 10/03/2017 16:14, Yang Zhong wrote:
> There is no need to delete subregion and do memory begin/commit for
> subpage in memory_region_finalize().
>
> This patch is from Anthony Xu <anthony...@intel.com>.
>
> Signed-off-by: Yang Zhong <yang.zh...@intel.com>
> ---
> memory.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index 284894b..3e9bfff 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1505,13 +1505,14 @@ 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);
> + if (!mr->subpage) {
> + 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();
Subpages never have subregions, so the loop never runs. The
begin/commit pair then becomes:
++memory_region_transaction_depth;
--memory_region_transaction_depth;
if (!memory_region_transaction_depth) {
if (memory_region_update_pending) {
...
} else if (ioeventfd_update_pending) {
...
}
// memory_region_clear_pending()
memory_region_update_pending = false;
ioeventfd_update_pending = false;
}
If memory_region_transaction_depth is > 0 the begin/commit pair does
nothing.
But if memory_region_transaction_depth is == 0, there should be no
update pending because the loop has never run. So I don't see what your
patch can change.
Of course there could be an update pending because of a bug elsewhere,
but I tried adding this patch:
diff --git a/memory.c b/memory.c
index 284894b..2208a21 100644
--- a/memory.c
+++ b/memory.c
@@ -903,6 +903,10 @@ static void
address_space_update_topology(AddressSpace *as)
void memory_region_transaction_begin(void)
{
qemu_flush_coalesced_mmio_buffer();
+ if (!memory_region_transaction_depth) {
+ assert(!memory_region_update_pending);
+ assert(!ioeventfd_update_pending);
+ }
++memory_region_transaction_depth;
}
and at least a basic qemu-system-x86_64 run started just fine. So why
does this patch make a difference?
Paolo
> }
> - memory_region_transaction_commit();
> -
