From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915/userptr: Hold mmref whilst calling get-user-pages Date: Tue, 5 Apr 2016 13:22:43 +0100 Message-ID: <5703AE13.80204@linux.intel.com> References: <1459703647-25795-1-git-send-email-chris@chris-wilson.co.uk> <1459703647-25795-2-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1459703647-25795-2-git-send-email-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On 03/04/16 18:14, Chris Wilson wrote: > Holding a reference to the containing task_struct is not sufficient t= o > prevent the mm_struct from being reaped under memory pressure. If thi= s > happens whilst we are calling get_user_pages(), explosions errupt - > sometimes an immediate GPF, sometimes page flag corruption. To preven= t > 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=C5=82 Winiarski > Cc: stable@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/dr= m/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 !=3D NULL) { > struct mm_struct *mm =3D obj->userptr.mm->mm; > > - down_read(&mm->mmap_sem); > - while (pinned < npages) { > - ret =3D 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 +=3D ret; > + ret =3D -EFAULT; > + if (atomic_inc_not_zero(&mm->mm_users)) { > + down_read(&mm->mmap_sem); > + while (pinned < npages) { > + ret =3D 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 +=3D ret; > + } > + up_read(&mm->mmap_sem); > + mmput(mm); > } > - up_read(&mm->mmap_sem); > } > > mutex_lock(&dev->struct_mutex); > Strange, doesn't this mean that the atomic_inc(¤t->mm->mm_count)=20 is not doing what we thought it would? Regards, Tvrtko