<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Feb 15, 2019 at 12:33 PM Koenig, Christian <<a href="mailto:Christian.Koenig@amd.com">Christian.Koenig@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">



<div bgcolor="#FFFFFF">
<div class="gmail-m_-4383219934271622999moz-cite-prefix">Am 15.02.19 um 19:16 schrieb Jason Ekstrand:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Feb 15, 2019 at 11:51 AM Christian König <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div class="gmail-m_-4383219934271622999gmail-m_5249618168223624859moz-cite-prefix">Am 15.02.19 um 17:49 schrieb Jason Ekstrand:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Feb 15, 2019 at 9:52 AM Lionel Landwerlin via dri-devel <<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 15/02/2019 14:32, Koenig, Christian wrote:<br>
> Am 15.02.19 um 15:23 schrieb Lionel Landwerlin:<br>
>> Hi Christian, David,<br>
>><br>
>> For timeline semaphore we need points to signaled in order.<br>
>> I'm struggling to understand how this fence-chain implementation<br>
>> preserves ordering of the seqnos.<br>
>><br>
>> One of the scenario I can see an issue happening is when you have a<br>
>> timeline with points 1 & 2 and userspace submits for 2 different<br>
>> engines :<br>
>>      - first with let's say a blitter style engine on point 2<br>
>>      - then a 3d style engine on point 1<br>
> Yeah, and where exactly is the problem?<br>
><br>
> Seqno 1 will signal when the 3d style engine finishes work.<br>
><br>
> And seqno 2 will signal when both seqno 1 is signaled and the blitter<br>
> style engine has finished its work.<br>
</blockquote>
<div><br>
</div>
<div>That's an interesting interpretation of the spec.  I think it's legal and I could see that behavior may be desirable in some ways.<br>
</div>
</div>
</div>
</blockquote>
<br>
Well we actually had this discussion multiple times now, both internally as well as on the mailing list. Please also see the previous mails with Daniel on this topic.<br>
</div>
</blockquote>
<div><br>
</div>
<div>I dug through dri-devel and read everything I could find with a search for "timeline semaphore"  I didn't find all that much but this did come up once.<br>
</div>
</div>
</div>
</blockquote>
<br>
Need to dig through my mails as well, that was back in November/December last year.<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">My initial suggestion was actually to exactly what Leonid suggested as well.<br>
<br>
And following this I used a rather simple container for the implementation, e.g. just a ring buffer indexed by the sequence number. In this scenario userspace can specify on syncobj creation time how big the window for sequence numbers should be, e.g. in this
 implementation how big the ring buffer would be.<br>
<br>
This was rejected by our guys who actually wrote a good part of the Vulkan specification. Daniel then has gone into the same direction during the public discussion.<br>
</div>
</blockquote>
<div><br>
</div>
<div>I agree with whoever said that specifying a ringbuffer size is unacceptable.  I'm not really sure how that's relevant though.  Is a ringbuffer required to implement the behavior that is being suggested here?  Genuine question; I'm trying to get back up
 to speed.<br>
</div>
</div>
</div>
</blockquote>
<br>
Using a ring buffer was just an example how we could do it if we follow my and Lionel's suggestion.<br>
<br>
Key point is that we could simplify the implementation massively if sequence numbers don't need to depend on each other.<br>
<br>
In other words we just see the syncobj as container where fences are added and retrieved from instead of something actively involved in the signaling.
<br></div></blockquote><div><br></div><div>In principal, I think this is a reasonable argument.  Having it involved in signalling doesn't seem terrible to me but it would mean that a driver wouldn't be able to detect that the fence it's waiting on actually belongs to itself and optimize things.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
Main reason we didn't do it this way is because the AMD Vulkan team has rejected this approach.<br></div></blockquote><div><br></div><div>Clearly, there's not quite as much agreement as I'd thought there was.  Oh, well, that's why we have these discussions.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
Additional to that chaining sequence numbers together is really the more defensive approach, e.g. it is less likely that applications can shoot themselves in the foot.<br></div></blockquote><div><br></div><div>Yeah, I can see how the "everything prior to n must be signalled" could be safer.  I think both wait-any and wait-all have their ups and downs.  It just took me by surprise.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> 4. If you do get into a sticky situation, you can unblock an entire timeline by using the CPU signal ioctl to set it to a high value.
<br>
</div>
</div>
</div>
</blockquote>
<br>
Well I think that this could be problematic as well. Keep in mind that main use case for this is sharing timelines between processes.<br>
<br>
In other words you don't want applications to be able to mess with it to much.<br>
</div>
</blockquote>
<div><br>
</div>
<div>Cross-process is exactly why you want it.  Suppose you're a compositor and you have a timeline shared with another application and you've submitted work which waits on it.  Then you get a notification somehow (SIGHUP?) that the client has died leaving
 you hanging.  What do you do?  You take the semaphore that's shared with you and the client and whack it to UINT64_MAX to unblock yourself.  Of course, this can be abused and that's always the risk you take with timelines.<br>
</div>
</div>
</div>
</blockquote>
<br>
My last status is that basically everybody agrees now that wait before signal in the kernel is forbidden.<br></div></blockquote><div><br></div><div>Agreed.  I'm not saying that wait before signal in the kernel should be a thing.  I think we're all agreed that wait-before-signal with the current GEM infrastructure is utter insanity.</div><div><br></div><div>However, timeline syncobjs are both a kernel wait mechanism (only for time points that already have a known-to-signal dma_fence) and a userspace wait mechanism (which can wait for things which haven't materialized yet).  The whacking to UINT64_MAX would be to unblock waiting userspace threads as you mentioned below.<br></div><div><br></div><div>That said, in the case that I suggested here, if the client process died, got kicked off the GPU, or whatever, then the kernel has likely declared it's context a loss and signalled all dma_fences associated with it.  If this is true, then whacking it to UINT64_MAX still works to unblock the timeline because there would be nothing else pending to signal time points.  Worst case, the process crashed and left valid GPU work pending in the kernel and the compositor ends up waiting a little while longer for said work to complete.<br></div><div><br></div><div>Ok, I think I'm reasonably convinced that the wait-all behaviour implied by the chaining approach, while unexpected, isn't harmful.</div><div><br></div><div>--Jason<br></div></div></div>