Re: [Qemu-devel] [PATCH] memory: Fix double unref of flatview

[Matthew Rosato]

On 02/12/2015 03:43 PM, Paolo Bonzini wrote:
> 
> 
> On 12/02/2015 20:32, Matthew Rosato wrote:
>> Could it be that the order in which flatview_unref (and therefore
>> memory_region_unref) vs object_unparent(mr) matters (ie, object_unparent
>> should always happen last)?  Prior to RCUification, seems like the
>> old_view was always unreferenced prior to return from
>> memory_region_del_subregion (so, memory_region_unref always occurred
>> before the object_unparent(mr)).  Now, the two could happen in either
>> order -- and looking at memory_region_unref, it is specifically looking
>> at the existence of obj->parent.
> 
> I think you're right.

FYI, then this probably also affects the places you hit in d8d9581460
"memory: convert memory_region_destroy to object_unparent", as that's
what I modeled this approach on -- but I haven't tested any of them.

> 
> To test your theory, you could try using call_rcu to unparent the memory
> region.  The complete fix would be to wrap the MemoryRegion within a

Yep, this test worked.

> container device, and do object_unparent on the container device.  Then
> the following happens:
> 
> 1) the container object's unparent method calls memory_region_del_subregion
> 
> 2) when the old flatview is destroyed, it calls memory_region_unref but
> the parent is still in place
> 
> 3) the container object is destroyed
> 
> 4) the container object destroys the container device.

OK great!  I noticed this problem while re-structuring this area of code
anyway, so I'll roll this in.

> 
> Note that these rules apply only to actual memory regions (I/O or RAM),
> not aliases or containers.  I hope to post a doc patch next week, since
> I'm completing the patches that fix the only other problematic case
> (PCIe MMCONFIG).  I was not expecting this to trigger so early, which is
> why I had left this for part 3.

Looking forward to it -- Thanks for the help Paolo.

Matt

> 
> Paolo
>