|
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
PaoloAlso, 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 ?
[Prev in Thread] | Current Thread | [Next in Thread] |