On Thu, Mar 17, 2016 at 10:09:28AM +0000, Tvrtko Ursulin wrote:
>
> On 17/03/16 09:37, Chris Wilson wrote:
> >Although the long term future of get_user_pages_locked() itself is
> >doubtful, the kernel currently recommends:
> >
> >/* get_user_pages should be phased out in favor of
> > * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
> > * should use get_user_pages because it cannot pass
> > * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
> > */
> >
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/i915_gem_userptr.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >index 54088a4d6498..fa7d8a8b66ef 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >@@ -500,20 +500,22 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
> > pvec = drm_malloc_ab(npages, sizeof(struct page *));
> > if (pvec != NULL) {
> > struct mm_struct *mm = obj->userptr.mm->mm;
> >+ int locked = 1;
> >
> > down_read(&mm->mmap_sem);
> > while (pinned < npages) {
> >- ret = get_user_pages(work->task, mm,
> >- obj->userptr.ptr + pinned * PAGE_SIZE,
> >- npages - pinned,
> >- !obj->userptr.read_only, 0,
> >- pvec + pinned, NULL);
> >+ ret = get_user_pages_locked(work->task, mm,
> >+ obj->userptr.ptr + pinned * PAGE_SIZE,
> >+ npages - pinned,
> >+ !obj->userptr.read_only, 0,
> >+ pvec + pinned, &locked);
> > if (ret < 0)
> > break;
> >
> > pinned += ret;
> > }
> >- up_read(&mm->mmap_sem);
> >+ if (locked)
> >+ up_read(&mm->mmap_sem);
> > }
> >
> > mutex_lock(&dev->struct_mutex);
> >
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Is it just cleanup or actually improves some real (as in hit by
> someone) scenario?
It's cleanup. Current open is
https://bugs.freedesktop.org/show_bug.cgi?id=93910
[ 5686.374255] WARNING: CPU: 0 PID: 13274 at mm/filemap.c:217
__delete_from_page_cache+0x274/0x280()
which may or may not be causing a concurrent GPF on the same page.
That concurrent GPF may or may not go away with the above patch. The
problem with indeterminant races!
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
