From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [PATCH 17/19] drm/radeon: use rcu waits in some ioctls Date: Mon, 04 Aug 2014 11:30:01 +0200 Message-ID: <53DF5299.2090305@canonical.com> References: <20140731153245.15061.63023.stgit@patser> <20140731153432.15061.49403.stgit@patser> <53DB4F5D.8000101@daenzer.net> <53DB680F.8000402@canonical.com> <53DBA076.2090503@daenzer.net> <53DBC96B.4010905@canonical.com> <53DF4761.7040109@daenzer.net> <53DF4ACA.7060402@canonical.com> <53DF5181.5060304@daenzer.net> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 827F16E2A2 for ; Mon, 4 Aug 2014 02:30:02 -0700 (PDT) In-Reply-To: <53DF5181.5060304@daenzer.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: =?windows-1252?Q?Michel_D=E4nzer?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org op 04-08-14 11:25, Michel D=E4nzer schreef: > On 04.08.2014 17:56, Maarten Lankhorst wrote: >> op 04-08-14 10:42, Michel D=E4nzer schreef: >>> On 02.08.2014 02:07, Maarten Lankhorst wrote: >>>> On 01-08-14 16:13, Michel D=E4nzer wrote: >>>>> On 01.08.2014 19:12, Maarten Lankhorst wrote: >>>>>> On 01-08-14 10:27, Michel D=E4nzer wrote: >>>>>>> On 01.08.2014 00:34, Maarten Lankhorst wrote: >>>>>>>> @@ -357,14 +360,20 @@ int radeon_gem_wait_idle_ioctl(struct drm_de= vice *dev, void *data, >>>>>>>> struct drm_radeon_gem_wait_idle *args =3D data; >>>>>>>> struct drm_gem_object *gobj; >>>>>>>> struct radeon_bo *robj; >>>>>>>> - int r; >>>>>>>> + int r =3D 0; >>>>>>>> + long ret; >>>>>>>> = >>>>>>>> gobj =3D drm_gem_object_lookup(dev, filp, args->handle); >>>>>>>> if (gobj =3D=3D NULL) { >>>>>>>> return -ENOENT; >>>>>>>> } >>>>>>>> robj =3D gem_to_radeon_bo(gobj); >>>>>>>> - r =3D radeon_bo_wait(robj, NULL, false); >>>>>>>> + ret =3D reservation_object_wait_timeout_rcu(robj->tbo.resv, true= , true, 30 * HZ); >>>>>>>> + if (ret =3D=3D 0) >>>>>>>> + r =3D -EBUSY; >>>>>>>> + else if (ret < 0) >>>>>>>> + r =3D ret; >>>>>>>> + >>>>>>>> /* callback hw specific functions if any */ >>>>>>>> if (rdev->asic->ioctl_wait_idle) >>>>>>>> robj->rdev->asic->ioctl_wait_idle(rdev, robj); >>>>>>> Heads up, this conflicts with >>>>>>> http://lists.freedesktop.org/archives/dri-devel/2014-August/065255.= html >>>>>>> which passes a non-NULL second argument to radeon_bo_wait() to get = the >>>>>>> BO's current domain. >>>>>> Ok, I will fix it up and resend it later. >>>>>> >>>>>> Does it matter if I grab the current domain without grabbing the lock >>>>>> here? Because it doesn't matter if it sees the old or new domain, it >>>>>> could have been changed after returning too. >>>>> It should be the domain where the BO is located when the fence we are >>>>> waiting for here signals. >>>> Could we compare domain before and after the rcu wait, and retry >>>> waiting if they're different, and the new one is VRAM? (eg eviction >>>> happened) That should prevent needing to lock the bo. >>> Eviction normally only happens from VRAM, not to VRAM. :) So if you know >>> whether the domain is VRAM or not after the wait, you can just proceed >>> accordingly, I don't see why you'd need to wait again. >> Because in the worst case you didn't wait on the fence that started >> the eviction, but one before it. ;-) > I'm afraid you've lost me. Can you determine the domain that > radeon_bo_wait() would have returned? > > Ok so.. wait ioctl: waits on fence 1 (eviction to vram happens asynchronously, fence 1 on the bo gets replaced b= y fence 2) wait 1 completes, new domain is VRAM vram flush happens, but fence 2 is not signaled yet so not everything is co= pied. wait ioctl returns Or is it unimportant here, and the vram flush doesn't depend on the fence b= eing completed? The second wait would be forced by ttm_bo_vm_fault anyway. ~Maarten