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