On 2018å¹´09æ??04æ?¥ 16:05, Christian König wrote: > Am 04.09.2018 um 09:53 schrieb zhoucm1: >> [SNIP] >>>>> >>>>> How about this idea: >>>>> 1. Each signaling point is a fence implementation with an rb node. >>>>> 2. Each node keeps a reference to the last previously inserted node. >>>>> 3. Each node is referenced by the sync object itself. >>>>> 4. Before each signal/wait operation we do a garbage collection >>>>> and remove the first node from the tree as long as it is signaled. >>>>> >>>>> 5. When enable_signaling is requested for a node we cascade that >>>>> to the left using rb_prev. >>>>>    This ensures that signaling is enabled for the current fence >>>>> as well as all previous fences. >>>>> >>>>> 6. A wait just looks into the tree for the signal point lower or >>>>> equal of the requested sequence number. >> After re-thought your idea, I think it doesn't work since there is no >> timeline value as a line: >> signal pt value doesn't must be continues, which can be jump by >> signal operation, like 1, 4, 8, 15, 19, e.g. there are five >> singal_pt, >> signal_pt1->signal_pt4->signal_pt8->signal_pt15->signal_pt19, if a >> wait pt is 7, do you mean this wait only needs signal_pt1 and >> signal_pt4??? That's certainly not right, we need to make sure the >> timeline value is bigger than wait pt value, that means signal_pt8 is >> need for wait_pt7. > > That can be defined as we like it, e.g. when a wait operation asks for > 7 we can return 8 as well. If defined this, then problem is coming again, if 8 is removed when garbage collection, you will return 15? Since there is no timeline as a line, I think this is not right direction. > > The key is that as soon as a signal point is added adding a previous > point is no longer allowed. That's intention. Regards, David Zhou > >>>>> >>>>> 7. When the sync object is released we use >>>>> rbtree_postorder_for_each_entry_safe() and drop the extra >>>>> reference to each node, but never call rb_erase! >>>>>    This way the rb_tree stays in memory, but without a root (e.g. >>>>> the sync object). It only destructs itself when the looked up >>>>> references to the nodes are dropped. >> And here, who will destroy rb node since no one do enable_signaling, >> and there is no callback to free themselves. > > The node will be destroyed when the last reference drops, not when > enable_signaling is called. > > In other words the sync_obj keeps the references to each tree object > to provide the wait operation, as soon as the sync_obj is destroyed we > don't need that functionality any more. > > We don't even need to wait for anything to be signaled, this way we > can drop all unused signal points as soon as the sync_obj is destroyed. > > Only the used ones will stay alive and provide the necessary > functionality to provide the signal for each wait operation. > > Regards, > Christian. > >> >> Regards, >> David Zhou >>>>> >>>>> Well that is quite a bunch of logic, but I think that should work >>>>> fine. >>>> Yeah, it could work, simple timeline reference also can solve >>>> 'free' problem. >>> >>> I think this approach is still quite a bit better, >> >>> e.g. you don't run into circle dependency problems, it needs less >>> memory and each node has always the same size which means we can use >>> a kmem_cache for it. >>> >>> Regards, >>> Christian. >>> >>>> >>>> Thanks, >>>> David Zhou >>> >> > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
