[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] memory: Fix double unref of flatview
From: |
Matthew Rosato |
Subject: |
Re: [Qemu-devel] [PATCH] memory: Fix double unref of flatview |
Date: |
Thu, 12 Feb 2015 22:29:50 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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
>