| Message ID | 20170223161830.26965-13-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 23, 2017 at 04:18:27PM +0000, Chris Wilson wrote: > This simply hides the EAGAIN caused by userptr when userspace causes > resource contention. However, it is quite beneficial with highly > contended userptr users as we avoid repeating the setup costs and > kernel-user context switches. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> -Michał > --- > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 10 +++++++++- > drivers/gpu/drm/i915/i915_gem.c | 4 +++- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++ > drivers/gpu/drm/i915/i915_gem_userptr.c | 18 +++++++++++++++--- > 5 files changed, 31 insertions(+), 5 deletions(-)
On Fri, Feb 24, 2017 at 02:53:07PM +0100, Michał Winiarski wrote: > On Thu, Feb 23, 2017 at 04:18:27PM +0000, Chris Wilson wrote: > > This simply hides the EAGAIN caused by userptr when userspace causes > > resource contention. However, it is quite beneficial with highly > > contended userptr users as we avoid repeating the setup costs and > > kernel-user context switches. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> Michał pointed out in irc that we need to hit the slowpath after failing to pin the userptr pages in the earlier reservation phase even if we have no relocation (flushing the wq inside the kernel is nicer than spinning on EAGAIN). (Hmm, with the exception that flush_workqueue is not interruptible. Hmm. I think this would be worth refining later to be an interruptible flush on the right set of userptr.) Anway, just to say that required changes to the preceding patch not to miss the slowpath on reservation failure. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 842c62b96a83..c04bd1058562 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -555,6 +555,7 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv) mutex_lock(&dev_priv->drm.struct_mutex); i915_gem_cleanup_engines(dev_priv); i915_gem_context_fini(dev_priv); + i915_gem_cleanup_userptr(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); i915_gem_drain_freed_objects(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 36c1d19a9fef..539a201359d2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1508,6 +1508,13 @@ struct i915_gem_mm { struct list_head fence_list; /** + * Workqueue to fault in userptr pages, flushed by the execbuf + * when required but otherwise left to userspace to try again + * on EAGAIN. + */ + struct workqueue_struct *userptr_wq; + + /** * Are we in a non-interruptible section of code like * modesetting? */ @@ -3170,7 +3177,8 @@ int i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); -void i915_gem_init_userptr(struct drm_i915_private *dev_priv); +int i915_gem_init_userptr(struct drm_i915_private *dev_priv); +void i915_gem_cleanup_userptr(struct drm_i915_private *dev_priv); int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file); int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4974b150bc3a..f46bad7680ec 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4510,7 +4510,9 @@ int i915_gem_init(struct drm_i915_private *dev_priv) */ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - i915_gem_init_userptr(dev_priv); + ret = i915_gem_init_userptr(dev_priv); + if (ret) + goto out_unlock; ret = i915_gem_init_ggtt(dev_priv); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index cdc57c1bd1db..8582a96b0e9b 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1377,6 +1377,9 @@ static int eb_relocate_slow(struct i915_execbuffer *eb) goto out; } + /* A frequent cause for EAGAIN are currently unavailable client pages */ + flush_workqueue(eb->i915->mm.userptr_wq); + ret = i915_mutex_lock_interruptible(dev); if (ret) { mutex_lock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 6a8fa085b74e..120186122c82 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -369,7 +369,7 @@ __i915_mm_struct_free(struct kref *kref) mutex_unlock(&mm->i915->mm_lock); INIT_WORK(&mm->work, __i915_mm_struct_free__worker); - schedule_work(&mm->work); + queue_work(mm->i915->mm.userptr_wq, &mm->work); } static void @@ -588,7 +588,7 @@ __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj, get_task_struct(work->task); INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); - schedule_work(&work->work); + queue_work(to_i915(obj->base.dev)->mm.userptr_wq, &work->work); *active = true; return ERR_PTR(-EAGAIN); @@ -816,8 +816,20 @@ i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file return 0; } -void i915_gem_init_userptr(struct drm_i915_private *dev_priv) +int i915_gem_init_userptr(struct drm_i915_private *dev_priv) { mutex_init(&dev_priv->mm_lock); hash_init(dev_priv->mm_structs); + + dev_priv->mm.userptr_wq = + alloc_workqueue("i915-userptr-acquire", WQ_HIGHPRI, 0); + if (!dev_priv->mm.userptr_wq) + return -ENOMEM; + + return 0; +} + +void i915_gem_cleanup_userptr(struct drm_i915_private *dev_priv) +{ + destroy_workqueue(dev_priv->mm.userptr_wq); }
This simply hides the EAGAIN caused by userptr when userspace causes resource contention. However, it is quite beneficial with highly contended userptr users as we avoid repeating the setup costs and kernel-user context switches. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 10 +++++++++- drivers/gpu/drm/i915/i915_gem.c | 4 +++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++ drivers/gpu/drm/i915/i915_gem_userptr.c | 18 +++++++++++++++--- 5 files changed, 31 insertions(+), 5 deletions(-)