[...] >> >>>>> 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 >> > If we consider the relationship of MemoryRegionImpl and device as the > one between file and file->private_data in Linux. Then the creation > of impl will object_ref(dev) and when impl->ref=0, it will > object_unref(dev) > But this is an async model, for those client which need to know the > end of flush, we can adopt callback just like call_rcu(). > > During this thread, it seems that no matter we adopt refcnt on MemoryRegionImpl or not, protecting MemoryRegion::opaque during dispatch is still required. It is challenging to substitute memory_region_init_io() 's 3rd parameter from void* to Object*, owning to the conflict that life-cycle management need the host of opaque, while Object and mr need 1:1 map. So I think, I will move forward on the way based on MemoryRegionOps::object(). Right?
Regards, pingfan > >> Regards, >> pingfan >>> -- >>> error compiling committee.c: too many arguments to function
