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. >> 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. > 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. What alternatives remain? -- error compiling committee.c: too many arguments to function
