On Mon, Sep 3, 2012 at 4:52 PM, Avi Kivity <a...@redhat.com> wrote: > On 09/03/2012 10:44 AM, liu ping fan wrote: >>>> >>> >>> If we make the refcount/lock internal to the region, we must remove the >>> opaque, since the region won't protect it. >>> >>> Replacing the opaque with container_of(mr) doesn't help, since we can't >>> refcount mr, only mr->impl. >>> >> I think you mean if using MemoryRegionImpl, then at this level, we had >> better not touch the refcnt of container_of(mr) and should face the >> mr->impl->refcnt. Right? > > I did not understand the second part, sorry. > My understanding of "Replacing the opaque with container_of(mr) doesn't help, since we can't refcount mr, only mr->impl." is that although Object_ref(container_of(mr)) can help us to protect it from disappearing. But apparently it is not right place to do it it in memory core. Do I catch you meaning?
>>> We could externalize the refcounting and push it into device code. This >>> means: >>> >>> memory_region_init_io(&s->mem, dev) >>> >>> ... >>> >>> object_ref(dev) >>> memory_region_add_subregion(..., &dev->mr) >>> >>> ... >>> >>> memory_region_del_subregion(..., &dev->mr) // implied flush >>> object_unref(dev) >>> >> I think "object_ref(dev)" just another style to push >> MemoryRegionOps::object() to device level. And I think we can bypass >> it. The caller (unplug, pci-reconfig ) of >> memory_region_del_subregion() ensure the @dev is valid. >> If the implied flush is implemented in synchronize, _del_subregion() >> will guarantee no reader for @dev->mr and @dev exist any longer. > > The above code has a deadlock. memory_region_del_subregion() may be > called under the device lock (since it may be the result of mmio to the > device), and if the flush uses synchronized_rcu(), it will wait forever > for the read-side critical section to complete. > But if _del_subregion() just wait for mr-X quiescent period, while calling in mr-Y's read side critical section, then we can avoid deadlock. I saw in pci-mapping, we delete mr-X in mr-Y read side. >> So I >> think we can save both object_ref/unref(dev) for memory system. >> The only problem is that whether we can implement it as synchronous or >> not, is it possible that we can launch a _del_subregion(mr-X) in >> mr-X's dispatcher? > > Yes. Real cases exist. Oh, I find the sample code, then, the deadlock is unavoidable in this method. > > What alternatives remain? > I think a way out may be async+refcnt Regards, pingfan > -- > error compiling committee.c: too many arguments to function