qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL v2 6/7] exec: allow to get a pointer for some mmi


From: KONRAD Frederic
Subject: Re: [Qemu-devel] [PULL v2 6/7] exec: allow to get a pointer for some mmio memory region
Date: Thu, 10 Aug 2017 15:23:48 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1



On 08/10/2017 02:59 PM, Paolo Bonzini wrote:
On 10/08/2017 14:56, Peter Maydell wrote:
+    qemu_mutex_lock_iothread();
+
+    /* Reset dirty so this doesn't happen later. */
+    cpu_physical_memory_test_and_clear_dirty(offset, size, 1);
+
+    if (section.mr != mr) {
+        /* memory_region_find add a ref on section.mr */
+        memory_region_unref(section.mr);
+        if (MMIO_INTERFACE(section.mr->owner)) {

Could somebody explain why it's OK to unref section.mr here before
we go on to do things with it, rather than only unrefing it after
we've finished using it?

The memory region won't disappear until you release the BQL and/or
RCU-read-lock, but yes it's cleaner to move it later, and yes there is a
leak.

I missed the case here where section.mr != mr but this shouldn't
happen. Either we make it acceptable and fix the leak.. or just
trigger an error as it is a bogus device. I'd rather go for the
first.
This is the same for memory_region_unref(section.mr).
memory_region_find must hit a MMIO_INTERFACE. In this case the
reference of MMIO_INTERFACE can't be zero here.

Thanks,
Fred


Paolo

Also, by my reading memory_region_find() will always ref
ret.mr (if it's not NULL), whereas this code only unrefs it
if section.mr == mr. Does this leak a reference in the case
where section.mr != mr, or am I missing something ?





reply via email to

[Prev in Thread] Current Thread [Next in Thread]