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: Fri, 01 Aug 2014 19:07:55 +0200 Message-ID: <53DBC96B.4010905@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> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mblankhorst.nl (mblankhorst.nl [141.105.120.124]) by gabe.freedesktop.org (Postfix) with ESMTP id A85FB6E7BA for ; Fri, 1 Aug 2014 10:08:04 -0700 (PDT) In-Reply-To: <53DBA076.2090503@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 On 01-08-14 16:13, Michel D=E4nzer wrote: > On 01.08.2014 19:12, Maarten Lankhorst wrote: >> Hey, >> >> 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_device= *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, tr= ue, 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 sh= ould prevent needing to lock the bo. ~Maarten