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 ?
