Re: [RFC v5 0/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

Hi, Peter

On 2023/2/22 下午11:57, Peter Xu wrote:
> On Wed, Feb 22, 2023 at 02:27:55PM +0800, Chuang Xu wrote:
> > Hi, Peter
> Hi, Chuang,
>
> > Note that as I mentioned in the comment, we temporarily replace this value
> > to prevent commit() and address_space_to_flatview() call each other recursively,
> > and eventually stack overflow.
> Sorry to have overlooked that part.  IMHO here it's not about the depth,
> but rather that we don't even need any RCU protection when updating
> ioeventfds because we exclusively own the FlatView* too there.
>
> I wanted to describe what I had in mind but instead I figured a patch may
> be needed to be accurate (with some cleanups alongside), hence attached.
> IIUC it can work with what I suggested before without fiddling with depth.
> Please have a look.  The patch should apply cleanly to master branch so if
> it works it can be your 1st patch too.
>
> PS: Paolo - I know I asked this before, but it'll be good to have your
> review comment on anything above.
>
> Thanks,
Here are two problems I can see:

1. It is inappropriate to use assert(qemu_mutex_iothread_locked()
&& !memory_region_update_pending) in update_ioeventfds().

For example, when entering commit(), if memory_region_update_pending
is true, the assertion will be triggered immediately when update_ioeventfds
is called.

2. The problem of stack overflow has not been solved. There are
too many places where address_space_to_flatview() may be called.

Here are another coredump stack:

#8  0x000055a3a769ed85 in memory_region_transaction_commit_force () at 
../softmmu/memory.c:1154
#9  0x000055a3a769fd75 in address_space_to_flatview (as=0x55a3a7ede180 
<address_space_memory>) at 
/data00/migration/qemu-open/include/exec/memory.h:1118
#10 address_space_update_topology_pass (as=as@entry=0x55a3a7ede180 
<address_space_memory>, old_view=old_view@entry=0x55a3a9d44990, 
new_view=new_view@entry=0x55a3d6837390,
    adding=adding@entry=false) at ../softmmu/memory.c:955
#11 0x000055a3a76a007c in address_space_set_flatview (as=as@entry=0x55a3a7ede180 
<address_space_memory>) at ../softmmu/memory.c:1062
#12 0x000055a3a769e870 in address_space_update_flatview_all () at 
../softmmu/memory.c:1107
#13 0x000055a3a769ed85 in memory_region_transaction_commit_force () at 
../softmmu/memory.c:1154
#14 0x000055a3a769fd75 in address_space_to_flatview (as=0x55a3a7ede180 
<address_space_memory>) at 
/data00/migration/qemu-open/include/exec/memory.h:1118
#15 address_space_update_topology_pass (as=as@entry=0x55a3a7ede180 
<address_space_memory>, old_view=old_view@entry=0x55a3a9d44990, 
new_view=new_view@entry=0x55a3d67f8d90,
    adding=adding@entry=false) at ../softmmu/memory.c:955
#16 0x000055a3a76a007c in address_space_set_flatview (as=as@entry=0x55a3a7ede180 
<address_space_memory>) at ../softmmu/memory.c:1062
#17 0x000055a3a769e870 in address_space_update_flatview_all () at 
../softmmu/memory.c:1107
#18 0x000055a3a769ed85 in memory_region_transaction_commit_force () at 
../softmmu/memory.c:1154
#19 0x000055a3a769fd75 in address_space_to_flatview (as=0x55a3a7ede180 
<address_space_memory>) at 
/data00/migration/qemu-open/include/exec/memory.h:1118
#20 address_space_update_topology_pass (as=as@entry=0x55a3a7ede180 
<address_space_memory>, old_view=old_view@entry=0x55a3a9d44990, 
new_view=new_view@entry=0x55a3d67ba790,
    adding=adding@entry=false) at ../softmmu/memory.c:955
#21 0x000055a3a76a007c in address_space_set_flatview (as=as@entry=0x55a3a7ede180 
<address_space_memory>) at ../softmmu/memory.c:1062
#22 0x000055a3a769e870 in address_space_update_flatview_all () at 
../softmmu/memory.c:1107
#23 0x000055a3a769ed85 in memory_region_transaction_commit_force () at 
../softmmu/memory.c:1154

And this may not be the only case where stack overflow occurs.
Thus, changing the depth value is the safest way I think.

Thanks!