On 08/30/2012 12:08 AM, Jan Kiszka wrote: > >>> > >>> We are dispatching according to the memory region (parameters, op > >>> handlers, opaques). If we end up in device object is not decided at this > >>> level. A memory region describes a dispatchable area - not to confuse > >>> with a device that may only partially be able to receive such requests. > >> > > But I think the meaning of the memory region is for dispatching. If no > > dispatching associated with mr, why need it exist in the system? > > Where did I say that memory regions should no longer be used for > dispatching? The point is to keep the clean layer separation between > memory regions and device objects instead of merging them together.
Believe me, that's exactly how I feel. I guess no author of a module wants to see it swallowed by a larger framework and see its design completely changed. Modules should be decoupled as much as possible. But I don't see a better way yet. > > Given > > Object > ^ ^ > | | > Region 1 Region 2 > > you protect the object during dispatch, implicitly (and that is bad) > requiring that no region must change in that period. We only protect the object against removal. After object_ref() has run, the region may still be disabled. > I say what rather > needs protection are the regions so that Region 2 can pass away and > maybe reappear independent of Region 1. Region 2 can go away until the device-supplied dispatch code takes the device lock. If the device chooses to use finer-grained locking, it can allow region 2 (or even region 1) to change during dispatch. The only requirement is that region 1 is not destroyed. > And: I won't need to know the > type of that object the regions are referring to in this model. That's > the difference. Sorry, I lost the reference. What model? Are you referring to my broken MemoryRegionImpl split? > > And > > could you elaborate that who will be the ref holder of mr? > > The memory subsystem while running a memory region handler. If that will > be a reference counter or a per-region lock like Avi suggested, we still > need to find out. > 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. 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) er, this must be wrong, since it's so simple -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.