Am 04.09.2018 um 11:00 schrieb zhoucm1: > > > 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? Well we of course keep what the last garbage collected number is, don't we? >>> 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. Yeah, but the syncobj can live for a very very long time. Not having some form of shrinking it when fences are signaled is certainly not going to fly very far. Regards, Christian. > > 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 >>> >> >
