From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Koenig, Christian" Subject: Re: Threaded submission & semaphore sharing Date: Fri, 2 Aug 2019 04:33:20 +0000 Message-ID: <970c65d1-ad63-4492-8ae2-96b603b2c7c8@email.android.com> References: , Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0223316488==" Return-path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-eopbgr780043.outbound.protection.outlook.com [40.107.78.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 77AD96E89E for ; Fri, 2 Aug 2019 04:33:22 +0000 (UTC) In-Reply-To: Content-Language: de-DE List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lionel Landwerlin Cc: Jason Ekstrand , dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0223316488== Content-Language: de-DE Content-Type: multipart/alternative; boundary="_000_970c65d1ad6344928ae296b603b2c7c8emailandroidcom_" --_000_970c65d1ad6344928ae296b603b2c7c8emailandroidcom_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Lionel, Well could you describe once more what the problem is? Cause I don't fully understand why a rather normal tandem submission with t= wo semaphores should fail in any way. Regards, Christian. Am 02.08.2019 06:28 schrieb Lionel Landwerlin : There aren't CTS tests covering the issue I was mentioning. But we could add them. I don't have all the details regarding your implementation but even with the "semaphore thread", I could see it running into the same issues. What if a mix of binary & timeline semaphores are handed to vkQueueSubmit()= ? For example with queueA & queueB from 2 different VkDevice : vkQueueSubmit(queueA, signal semA); vkQueueSubmit(queueA, wait on [semA, timelineSemB]); with timelineSemB triggering a wait before signal. vkQueueSubmit(queueB, signal semA); -Lionel On 02/08/2019 06:18, Zhou, David(ChunMing) wrote: > Hi Lionel, > > By the Queue thread is a heavy thread, which is always resident in driver= during application running, our guys don't like that. So we switch to Sema= phore Thread, only when waitBeforeSignal of timeline happens, we spawn a th= read to handle that wait. So we don't have your this issue. > By the way, I already pass all your CTS cases for now. I suggest you to s= witch to Semaphore Thread instead of Queue Thread as well. It works very we= ll. > > -David > > -----Original Message----- > From: Lionel Landwerlin > Sent: Friday, August 2, 2019 4:52 AM > To: dri-devel ; Koenig, Christian ; Zhou, David(ChunMing) ; Jason Ek= strand > Subject: Threaded submission & semaphore sharing > > Hi Christian, David, > > Sorry to report this so late in the process, but I think we found an issu= e not directly related to syncobj timelines themselves but with a side effe= ct of the threaded submissions. > > Essentially we're failing a test in crucible : > func.sync.semaphore-fd.opaque-fd > This test create a single binary semaphore, shares it between 2 VkDevice/= VkQueue. > Then in a loop it proceeds to submit workload alternating between the 2 V= kQueue with one submit depending on the other. > It does so by waiting on the VkSemaphore signaled in the previous iterati= on and resignaling it. > > The problem for us is that once things are dispatched to the submission t= hread, the ordering of the submission is lost. > Because we have 2 devices and they both have their own submission thread. > > Jason suggested that we reestablish the ordering by having semaphores/syn= cobjs carry an additional uint64_t payload. > This 64bit integer would represent be an identifier that submission threa= ds will WAIT_FOR_AVAILABLE on. > > The scenario would look like this : > - vkQueueSubmit(queueA, signal on semA); > - in the caller thread, this would increment the syncobj additi= onal u64 payload and return it to userspace. > - at some point the submission thread of queueA submits the wor= kload and signal the syncobj of semA with value returned in the caller thre= ad of vkQueueSubmit(). > - vkQueueSubmit(queueB, wait on semA); > - in the caller thread, this would read the syncobj additional > u64 payload > - at some point the submission thread of queueB will try to sub= mit the work, but first it will WAIT_FOR_AVAILABLE the u64 value returned i= n the step above > > Because we want the binary semaphores to be shared across processes and w= ould like this to remain a single FD, the simplest location to store this a= dditional u64 payload would be the DRM syncobj. > It would need an additional ioctl to read & increment the value. > > What do you think? > > -Lionel --_000_970c65d1ad6344928ae296b603b2c7c8emailandroidcom_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable
Hi Lionel,

Well could you describe once more what the problem is?

Cause I don't fully understand why a rather normal tandem= submission with two semaphores should fail in any way.

Regards,
Christian.

Am 02.08.2019 06:28 schrieb Lionel Landwerlin = <lionel.g.landwerlin@intel.com>:
There aren't CTS tests covering the issue I was me= ntioning.
But we could add them.

I don't have all the details regarding your implementation but even with the "semaphore thread", I could see it running into the same issu= es.
What if a mix of binary & timeline semaphores are handed to vkQueueSubm= it()?

For example with queueA & queueB from 2 different VkDevice :
     vkQueueSubmit(queueA, signal semA);
     vkQueueSubmit(queueA, wait on [semA, timelineSemB]= ); with
timelineSemB triggering a wait before signal.
     vkQueueSubmit(queueB, signal semA);


-Lionel

On 02/08/2019 06:18, Zhou, David(ChunMing) wrote:
> Hi Lionel,
>
> By the Queue thread is a heavy thread, which is always resident in dri= ver during application running, our guys don't like that. So we switch to S= emaphore Thread, only when waitBeforeSignal of timeline happens, we spawn a= thread to handle that wait. So we don't have your this issue.
> By the way, I already pass all your CTS cases for now. I suggest you t= o switch to Semaphore Thread instead of Queue Thread as well. It works very= well.
>
> -David
>
> -----Original Message-----
> From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Sent: Friday, August 2, 2019 4:52 AM
> To: dri-devel <dri-devel@lists.freedesktop.org>; Koenig, Christi= an <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@= amd.com>; Jason Ekstrand <jason@jlekstrand.net>
> Subject: Threaded submission & semaphore sharing
>
> Hi Christian, David,
>
> Sorry to report this so late in the process, but I think we found an i= ssue not directly related to syncobj timelines themselves but with a side e= ffect of the threaded submissions.
>
> Essentially we're failing a test in crucible :
> func.sync.semaphore-fd.opaque-fd
> This test create a single binary semaphore, shares it between 2 VkDevi= ce/VkQueue.
> Then in a loop it proceeds to submit workload alternating between the = 2 VkQueue with one submit depending on the other.
> It does so by waiting on the VkSemaphore signaled in the previous iter= ation and resignaling it.
>
> The problem for us is that once things are dispatched to the submissio= n thread, the ordering of the submission is lost.
> Because we have 2 devices and they both have their own submission thre= ad.
>
> Jason suggested that we reestablish the ordering by having semaphores/= syncobjs carry an additional uint64_t payload.
> This 64bit integer would represent be an identifier that submission th= reads will WAIT_FOR_AVAILABLE on.
>
> The scenario would look like this :
>       - vkQueueSubmit(queueA, signal on semA)= ;
>           - in the caller thre= ad, this would increment the syncobj additional u64 payload and return it t= o userspace.
>           - at some point the = submission thread of queueA submits the workload and signal the syncobj of = semA with value returned in the caller thread of vkQueueSubmit().
>       - vkQueueSubmit(queueB, wait on semA);<= br> >           - in the caller thre= ad, this would read the syncobj additional
> u64 payload
>           - at some point the = submission thread of queueB will try to submit the work, but first it will = WAIT_FOR_AVAILABLE the u64 value returned in the step above
>
> Because we want the binary semaphores to be shared across processes an= d would like this to remain a single FD, the simplest location to store thi= s additional u64 payload would be the DRM syncobj.
> It would need an additional ioctl to read & increment the value. >
> What do you think?
>
> -Lionel


 --_000_970c65d1ad6344928ae296b603b2c7c8emailandroidcom_-- --===============0223316488== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============0223316488==--