Re: [Qemu-devel] [PATCH v7 0/7] push mmio dispatch out of big lock

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2013-05-06 12:58, Paolo Bonzini wrote:
> Il 06/05/2013 12:56, Jan Kiszka ha scritto:
>>> The problem is that even if I/O for a region is supposed to happen
>>> within the BQL, lookup can happen outside the BQL.  Lookup will use the
>>> region even if it is just to discard it:
>>>
>>>            VCPU thread (under BQL)              device thread
>>>  --------------------------------------------------------------------------------------
>>>                                                 flatview_ref
>>>                                                 memory_region_find returns d->mr
>>>                                                 memory_region_ref(d->mr) /* nop */
>>>            qdev_free(d)
>>>              object_unparent(d)
>>>                unrealize(d)
>>>                  memory_region_del_subregion(d->mr)
>>>                    FlatView updated, d->mr not in the new view
>>>
>>>                                                 flatview_unref
>>>                                                   memory_region_unref(d->mr)
>>>                                                     object_unref(d)
>>>                                                       free(d)
>>>                                                 if (!d->mr->is_ram) {        /* BAD! */
>>>                                                   memory_region_unref(d->mr) /* nop */
>>>                                                   return error
>>>                                                 }
>>>
>>>
>>> Here, the memory region is dereferenced *before* we know that it is BQL-free
>>> (in fact, exactly to ascertain whether it is BQL-free).
>>
>> Both flatview update and lookup *plus* locking type evaluation (i.e.
>> memory region dereferencing) always happen under the address space lock.
>> See Pingfan's patch.
> 
> That's true of address_space_rw/map, but I don't think it holds for
> memory_region_find.

It has to, or it would be broken: Either it is called on a region that
supports reference counting and, thus, increments the counter before
returning, or it has to be called with the BQL held.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux