On 2018å¹´09æ??04æ?¥ 16:42, Christian König wrote: > Am 04.09.2018 um 10:27 schrieb zhoucm1: >> >> >> 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? > > The garbage collection is only done for signaled nodes. So when 8 is > already garbage collected and 7 is asked we know that we don't need to > return anything. 8 is a signaled node, waitA/signal operation do garbage collection, how waitB(7) know the garbage history? > >> Since there is no timeline as a line, I think this is not right >> direction. > > That is actually intended. There is no infinite timeline here, just a > windows of the last not yet signaled fences. No one said the it's a infinite timeline, timeline will stop increasing when syncobj is released. Anyway kref is a good way to solve the 'free' problem, I will try to use it improve my patch, of course, will refer your idea.:) Thanks, David Zhou > > Otherwise you will never be able to release nodes from the tree since > you always need to keep them around just in case somebody asks for a > lower number. > > Regards, > Christian. > >> >>> >>> 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 >> >
