On 2018å¹´09æ??04æ?¥ 17:20, Christian König wrote: > 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. I will try to fix this problem. btw, when I try your suggestion, I find it will be difficult to implement drm_syncobj_array_wait_timeout by your idea, since it needs first_signaled. if there is un-signaled syncobj, we will still register cb to timeline value change, then still back to need enble_signaling. Thanks, David Zhou > > 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 >>>> >>> >> > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
