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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
