| Message ID | 1435881253-4912-1-git-send-email-greg@chown.ath.cx (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03.07.2015 01:54, Grigori Goronzy wrote: > We don't need to call the (expensive) radeon_bo_wait, checking the > fences via RCU is much faster. The reservation done by radeon_bo_wait > does not save us from any race conditions. > > Signed-off-by: Grigori Goronzy <greg@chown.ath.cx> Patche #1-#3 are Reviewed-by: Christian König <christian.koenig@amd.com> Patch #4 is a nice catch, but as Michel already noted needs more thoughts for a complete fix. Regards, Christian. > --- > drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c > index ac3c131..7199e19 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -428,7 +428,6 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data, > int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > { > - struct radeon_device *rdev = dev->dev_private; > struct drm_radeon_gem_busy *args = data; > struct drm_gem_object *gobj; > struct radeon_bo *robj; > @@ -440,10 +439,16 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, > return -ENOENT; > } > robj = gem_to_radeon_bo(gobj); > - r = radeon_bo_wait(robj, &cur_placement, true); > + > + r = reservation_object_test_signaled_rcu(robj->tbo.resv, true); > + if (r == 0) > + r = -EBUSY; > + else > + r = 0; > + > + cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type); > args->domain = radeon_mem_type_to_domain(cur_placement); > drm_gem_object_unreference_unlocked(gobj); > - r = radeon_gem_handle_lockup(rdev, r); > return r; > } >
On Fri, Jul 3, 2015 at 5:45 AM, Christian König <deathsimple@vodafone.de> wrote: > On 03.07.2015 01:54, Grigori Goronzy wrote: >> >> We don't need to call the (expensive) radeon_bo_wait, checking the >> fences via RCU is much faster. The reservation done by radeon_bo_wait >> does not save us from any race conditions. >> >> Signed-off-by: Grigori Goronzy <greg@chown.ath.cx> > > > Patche #1-#3 are Reviewed-by: Christian König <christian.koenig@amd.com> > Applied 1-3. thanks! Alex > Patch #4 is a nice catch, but as Michel already noted needs more thoughts > for a complete fix. > > Regards, > Christian. > > >> --- >> drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c >> b/drivers/gpu/drm/radeon/radeon_gem.c >> index ac3c131..7199e19 100644 >> --- a/drivers/gpu/drm/radeon/radeon_gem.c >> +++ b/drivers/gpu/drm/radeon/radeon_gem.c >> @@ -428,7 +428,6 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void >> *data, >> int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, >> struct drm_file *filp) >> { >> - struct radeon_device *rdev = dev->dev_private; >> struct drm_radeon_gem_busy *args = data; >> struct drm_gem_object *gobj; >> struct radeon_bo *robj; >> @@ -440,10 +439,16 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, >> void *data, >> return -ENOENT; >> } >> robj = gem_to_radeon_bo(gobj); >> - r = radeon_bo_wait(robj, &cur_placement, true); >> + >> + r = reservation_object_test_signaled_rcu(robj->tbo.resv, true); >> + if (r == 0) >> + r = -EBUSY; >> + else >> + r = 0; >> + >> + cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type); >> args->domain = radeon_mem_type_to_domain(cur_placement); >> drm_gem_object_unreference_unlocked(gobj); >> - r = radeon_gem_handle_lockup(rdev, r); >> return r; >> } >> > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index ac3c131..7199e19 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -428,7 +428,6 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data, int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) { - struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_busy *args = data; struct drm_gem_object *gobj; struct radeon_bo *robj; @@ -440,10 +439,16 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, return -ENOENT; } robj = gem_to_radeon_bo(gobj); - r = radeon_bo_wait(robj, &cur_placement, true); + + r = reservation_object_test_signaled_rcu(robj->tbo.resv, true); + if (r == 0) + r = -EBUSY; + else + r = 0; + + cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type); args->domain = radeon_mem_type_to_domain(cur_placement); drm_gem_object_unreference_unlocked(gobj); - r = radeon_gem_handle_lockup(rdev, r); return r; }
We don't need to call the (expensive) radeon_bo_wait, checking the fences via RCU is much faster. The reservation done by radeon_bo_wait does not save us from any race conditions. Signed-off-by: Grigori Goronzy <greg@chown.ath.cx> --- drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)