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

From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	KONRAD Frederic <frederic.konrad@adacore.com>,
	Richard Henderson <rth@twiddle.net>,
	Edgar Iglesias <edgar.iglesias@xilinx.com>
Subject: Re: [Qemu-devel] [PULL v2 6/7] exec: allow to get a pointer for some mmio memory region
Date: Thu, 10 Aug 2017 14:59:34 +0200	[thread overview]
Message-ID: <9fc966eb-ee33-d5b8-fef8-5472853ca4fc@redhat.com> (raw)
In-Reply-To: <CAFEAcA_7_EwgxfnEmr+moHc5u4uprKjyL+ZA1VzspYwqONjfCg@mail.gmail.com>

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.

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 ?
> 