| Message ID | 1449047626-11168-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 02, 2015 at 09:13:46AM +0000, Chris Wilson wrote: > In commit 2e1b873072dfe3bbcc158a9c21acde1ab0d36c55 [v4.2] > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Mon Apr 27 13:41:22 2015 +0100 > > drm/i915: Convert RPS tracking to a intel_rps_client struct > > we converted the __i915_wait_request() to take a new intel_rps_client > struct (rather than having to pass fake drm_i915_file_private structs). > However, due to use of passing a void pointer, I didn't spot one > callsite in wait-ioctl was passing the wrong pointer. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: stable@vger.kernel.org Fwiw, the impact of this bug is zero. Along the rps path, we always first call list_empty(rps) which when we pass in the wrong pointer always evaluates to false and we return early and never chase the invalid pointers. The user visible impact is then wait-ioctl doesn't get the same waitboosting as the other interfaces (set-domain, throttle), which is a performance concern for the *very* few users of the wait interface. There is also a libdrm_intel patch to use the wait-ioctl for drm_intel_bo_wait_rendering() if anyone feels inclined to review libdrm_intel patches. -Chris
On Thu, Dec 03, 2015 at 09:25:37PM +0000, Chris Wilson wrote: > On Wed, Dec 02, 2015 at 09:13:46AM +0000, Chris Wilson wrote: > > In commit 2e1b873072dfe3bbcc158a9c21acde1ab0d36c55 [v4.2] > > Author: Chris Wilson <chris@chris-wilson.co.uk> > > Date: Mon Apr 27 13:41:22 2015 +0100 > > > > drm/i915: Convert RPS tracking to a intel_rps_client struct > > > > we converted the __i915_wait_request() to take a new intel_rps_client > > struct (rather than having to pass fake drm_i915_file_private structs). > > However, due to use of passing a void pointer, I didn't spot one > > callsite in wait-ioctl was passing the wrong pointer. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: stable@vger.kernel.org > > Fwiw, the impact of this bug is zero. Along the rps path, we always > first call list_empty(rps) which when we pass in the wrong pointer > always evaluates to false and we return early and never chase the > invalid pointers. > > The user visible impact is then wait-ioctl doesn't get the same > waitboosting as the other interfaces (set-domain, throttle), which is a > performance concern for the *very* few users of the wait interface. > There is also a libdrm_intel patch to use the wait-ioctl for > drm_intel_bo_wait_rendering() if anyone feels inclined to review > libdrm_intel patches. I added this to the commit message and figured it's not 100% justified for -fixes. So applied to dinq. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f5aef48b93db..cd8c4024af92 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3081,7 +3081,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) if (ret == 0) ret = __i915_wait_request(req[i], true, args->timeout_ns > 0 ? &args->timeout_ns : NULL, - file->driver_priv); + to_rps_client(file)); i915_gem_request_unreference__unlocked(req[i]); } return ret;
In commit 2e1b873072dfe3bbcc158a9c21acde1ab0d36c55 [v4.2] Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Mon Apr 27 13:41:22 2015 +0100 drm/i915: Convert RPS tracking to a intel_rps_client struct we converted the __i915_wait_request() to take a new intel_rps_client struct (rather than having to pass fake drm_i915_file_private structs). However, due to use of passing a void pointer, I didn't spot one callsite in wait-ioctl was passing the wrong pointer. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)