Re: [Intel-gfx] [PATCH 2/3] drm/i915/userptr: Hold mmref whilst calling get-user-pages
On 03/04/16 18:14, Chris Wilson wrote: Holding a reference to the containing task_struct is not sufficient to prevent the mm_struct from being reaped under memory pressure. If this happens whilst we are calling get_user_pages(), explosions errupt - sometimes an immediate GPF, sometimes page flag corruption. To prevent the target mm from being reaped as we are reading from it, acquire a reference before we begin. Testcase: igt/gem_shrink/*userptr Signed-off-by: Chris WilsonCc: Tvrtko Ursulin Cc: Michał Winiarski Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_gem_userptr.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 92b39186b05a..960bb37f458f 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -547,19 +547,24 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) if (pvec != NULL) { struct mm_struct *mm = obj->userptr.mm->mm; - down_read(>mmap_sem); - while (pinned < npages) { - ret = get_user_pages_remote(work->task, mm, - obj->userptr.ptr + pinned * PAGE_SIZE, - npages - pinned, - !obj->userptr.read_only, 0, - pvec + pinned, NULL); - if (ret < 0) - break; - - pinned += ret; + ret = -EFAULT; + if (atomic_inc_not_zero(>mm_users)) { + down_read(>mmap_sem); + while (pinned < npages) { + ret = get_user_pages_remote + (work->task, mm, +obj->userptr.ptr + pinned * PAGE_SIZE, +npages - pinned, +!obj->userptr.read_only, 0, +pvec + pinned, NULL); + if (ret < 0) + break; + + pinned += ret; + } + up_read(>mmap_sem); + mmput(mm); } - up_read(>mmap_sem); } mutex_lock(>struct_mutex); Strange, doesn't this mean that the atomic_inc(>mm->mm_count) is not doing what we thought it would? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/userptr: Hold mmref whilst calling get-user-pages
On Sun, Apr 03, 2016 at 06:14:06PM +0100, Chris Wilson wrote: > Holding a reference to the containing task_struct is not sufficient to > prevent the mm_struct from being reaped under memory pressure. If this > happens whilst we are calling get_user_pages(), explosions errupt - > sometimes an immediate GPF, sometimes page flag corruption. To prevent > the target mm from being reaped as we are reading from it, acquire a > reference before we begin. > > Testcase: igt/gem_shrink/*userptr > Signed-off-by: Chris Wilson> Cc: Tvrtko Ursulin > Cc: Michał Winiarski > Cc: sta...@vger.kernel.org Reviewed-by: Michał Winiarski -Michał > --- > drivers/gpu/drm/i915/i915_gem_userptr.c | 29 + > 1 file changed, 17 insertions(+), 12 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/userptr: Hold mmref whilst calling get-user-pages
Holding a reference to the containing task_struct is not sufficient to prevent the mm_struct from being reaped under memory pressure. If this happens whilst we are calling get_user_pages(), explosions errupt - sometimes an immediate GPF, sometimes page flag corruption. To prevent the target mm from being reaped as we are reading from it, acquire a reference before we begin. Testcase: igt/gem_shrink/*userptr Signed-off-by: Chris WilsonCc: Tvrtko Ursulin Cc: Michał Winiarski Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_gem_userptr.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 92b39186b05a..960bb37f458f 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -547,19 +547,24 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) if (pvec != NULL) { struct mm_struct *mm = obj->userptr.mm->mm; - down_read(>mmap_sem); - while (pinned < npages) { - ret = get_user_pages_remote(work->task, mm, - obj->userptr.ptr + pinned * PAGE_SIZE, - npages - pinned, - !obj->userptr.read_only, 0, - pvec + pinned, NULL); - if (ret < 0) - break; - - pinned += ret; + ret = -EFAULT; + if (atomic_inc_not_zero(>mm_users)) { + down_read(>mmap_sem); + while (pinned < npages) { + ret = get_user_pages_remote + (work->task, mm, +obj->userptr.ptr + pinned * PAGE_SIZE, +npages - pinned, +!obj->userptr.read_only, 0, +pvec + pinned, NULL); + if (ret < 0) + break; + + pinned += ret; + } + up_read(>mmap_sem); + mmput(mm); } - up_read(>mmap_sem); } mutex_lock(>struct_mutex); -- 2.8.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx