| Message ID | 20240105184624.508603-10-dmitry.osipenko@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers | expand |
On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: > Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation > lock if pages_use_count is non-zero, leveraging from atomicity of the > refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. > > Acked-by: Maxime Ripard <mripard@kernel.org> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index cacf0f8c42e2..1c032513abf1 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); > > +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > +{ > + int ret; Just random drive-by comment: a might_lock annotation here might be good, or people could hit some really interesting bugs that are rather hard to reproduce ... -Sima > + > + if (refcount_inc_not_zero(&shmem->pages_use_count)) > + return 0; > + > + dma_resv_lock(shmem->base.resv, NULL); > + ret = drm_gem_shmem_get_pages_locked(shmem); > + dma_resv_unlock(shmem->base.resv); > + > + return ret; > +} > + > static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem) > { > int ret; > @@ -609,10 +623,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct > return ret; > } > > - dma_resv_lock(shmem->base.resv, NULL); > - ret = drm_gem_shmem_get_pages_locked(shmem); > - dma_resv_unlock(shmem->base.resv); > - > + ret = drm_gem_shmem_get_pages(shmem); > if (ret) > return ret; > > -- > 2.43.0 >
On 1/25/24 20:24, Daniel Vetter wrote: > On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: >> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation >> lock if pages_use_count is non-zero, leveraging from atomicity of the >> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. >> >> Acked-by: Maxime Ripard <mripard@kernel.org> >> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> >> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> >> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> --- >> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >> index cacf0f8c42e2..1c032513abf1 100644 >> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) >> } >> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); >> >> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) >> +{ >> + int ret; > > Just random drive-by comment: a might_lock annotation here might be good, > or people could hit some really interesting bugs that are rather hard to > reproduce ... > -Sima Thanks for the suggestion!
On Thu, 25 Jan 2024 18:24:04 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: > > Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation > > lock if pages_use_count is non-zero, leveraging from atomicity of the > > refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. > > > > Acked-by: Maxime Ripard <mripard@kernel.org> > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > > Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > --- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index cacf0f8c42e2..1c032513abf1 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) > > } > > EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); > > > > +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > > +{ > > + int ret; > > Just random drive-by comment: a might_lock annotation here might be good, > or people could hit some really interesting bugs that are rather hard to > reproduce ... Actually, being able to acquire a ref in a dma-signalling path on an object we know for sure already has refcount >= 1 (because we previously acquired a ref in a path where dma_resv_lock() was allowed), was the primary reason I suggested moving to this atomic-refcount approach. In the meantime, drm_gpuvm has evolved in a way that allows me to not take the ref in the dma-signalling path (the gpuvm_bo object now holds the ref, and it's acquired/released outside the dma-signalling path). Not saying we shouldn't add this might_lock(), but others might have good reasons to have this function called in a path where locking is not allowed.
On 1/26/24 13:18, Boris Brezillon wrote: > On Thu, 25 Jan 2024 18:24:04 +0100 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: >>> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation >>> lock if pages_use_count is non-zero, leveraging from atomicity of the >>> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. >>> >>> Acked-by: Maxime Ripard <mripard@kernel.org> >>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> >>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> >>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >>> --- >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- >>> 1 file changed, 15 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> index cacf0f8c42e2..1c032513abf1 100644 >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) >>> } >>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); >>> >>> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) >>> +{ >>> + int ret; >> >> Just random drive-by comment: a might_lock annotation here might be good, >> or people could hit some really interesting bugs that are rather hard to >> reproduce ... > > Actually, being able to acquire a ref in a dma-signalling path on an > object we know for sure already has refcount >= 1 (because we previously > acquired a ref in a path where dma_resv_lock() was allowed), was the > primary reason I suggested moving to this atomic-refcount approach. > > In the meantime, drm_gpuvm has evolved in a way that allows me to not > take the ref in the dma-signalling path (the gpuvm_bo object now holds > the ref, and it's acquired/released outside the dma-signalling path). > > Not saying we shouldn't add this might_lock(), but others might have > good reasons to have this function called in a path where locking > is not allowed. For Panthor the might_lock indeed won't be a appropriate, thanks for reminding about it. I'll add explanatory comment to the code.
On Fri, Jan 26, 2024 at 07:43:29PM +0300, Dmitry Osipenko wrote: > On 1/26/24 13:18, Boris Brezillon wrote: > > On Thu, 25 Jan 2024 18:24:04 +0100 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > >> On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: > >>> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation > >>> lock if pages_use_count is non-zero, leveraging from atomicity of the > >>> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. > >>> > >>> Acked-by: Maxime Ripard <mripard@kernel.org> > >>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > >>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > >>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > >>> --- > >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- > >>> 1 file changed, 15 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > >>> index cacf0f8c42e2..1c032513abf1 100644 > >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > >>> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) > >>> } > >>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); > >>> > >>> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > >>> +{ > >>> + int ret; > >> > >> Just random drive-by comment: a might_lock annotation here might be good, > >> or people could hit some really interesting bugs that are rather hard to > >> reproduce ... > > > > Actually, being able to acquire a ref in a dma-signalling path on an > > object we know for sure already has refcount >= 1 (because we previously > > acquired a ref in a path where dma_resv_lock() was allowed), was the > > primary reason I suggested moving to this atomic-refcount approach. > > > > In the meantime, drm_gpuvm has evolved in a way that allows me to not > > take the ref in the dma-signalling path (the gpuvm_bo object now holds > > the ref, and it's acquired/released outside the dma-signalling path). > > > > Not saying we shouldn't add this might_lock(), but others might have > > good reasons to have this function called in a path where locking > > is not allowed. > > For Panthor the might_lock indeed won't be a appropriate, thanks for > reminding about it. I'll add explanatory comment to the code. Hm these kind of tricks feel very dangerous to me. I think it would be good to split up the two cases into two functions: 1. first one does only the atomic_inc and splats if the refcount is zero. I think something in the name that denotes that we're incrementing a borrowed pages reference would be good here, so like get_borrowed_pages (there's not really a naming convention for these in the kernel). Unfortunately no rust so we can't enforce that you provide the right kind of borrowed reference at compile time. 2. second one has the might_lock. This way you force callers to think what they're doing and ideally document where the borrowed reference is from, and ideally document that in the code. Otherwise we'll end up with way too much "works in testing, but is a nice CVE" code :-/ Cheers, Sima
On 1/30/24 11:34, Daniel Vetter wrote: > On Fri, Jan 26, 2024 at 07:43:29PM +0300, Dmitry Osipenko wrote: >> On 1/26/24 13:18, Boris Brezillon wrote: >>> On Thu, 25 Jan 2024 18:24:04 +0100 >>> Daniel Vetter <daniel@ffwll.ch> wrote: >>> >>>> On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: >>>>> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation >>>>> lock if pages_use_count is non-zero, leveraging from atomicity of the >>>>> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. >>>>> >>>>> Acked-by: Maxime Ripard <mripard@kernel.org> >>>>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> >>>>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> >>>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >>>>> --- >>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- >>>>> 1 file changed, 15 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> index cacf0f8c42e2..1c032513abf1 100644 >>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) >>>>> } >>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); >>>>> >>>>> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) >>>>> +{ >>>>> + int ret; >>>> >>>> Just random drive-by comment: a might_lock annotation here might be good, >>>> or people could hit some really interesting bugs that are rather hard to >>>> reproduce ... >>> >>> Actually, being able to acquire a ref in a dma-signalling path on an >>> object we know for sure already has refcount >= 1 (because we previously >>> acquired a ref in a path where dma_resv_lock() was allowed), was the >>> primary reason I suggested moving to this atomic-refcount approach. >>> >>> In the meantime, drm_gpuvm has evolved in a way that allows me to not >>> take the ref in the dma-signalling path (the gpuvm_bo object now holds >>> the ref, and it's acquired/released outside the dma-signalling path). >>> >>> Not saying we shouldn't add this might_lock(), but others might have >>> good reasons to have this function called in a path where locking >>> is not allowed. >> >> For Panthor the might_lock indeed won't be a appropriate, thanks for >> reminding about it. I'll add explanatory comment to the code. > > Hm these kind of tricks feel very dangerous to me. I think it would be > good to split up the two cases into two functions: > > 1. first one does only the atomic_inc and splats if the refcount is zero. > I think something in the name that denotes that we're incrementing a > borrowed pages reference would be good here, so like get_borrowed_pages > (there's not really a naming convention for these in the kernel). > Unfortunately no rust so we can't enforce that you provide the right kind > of borrowed reference at compile time. > > 2. second one has the might_lock. > > This way you force callers to think what they're doing and ideally > document where the borrowed reference is from, and ideally document that > in the code. Otherwise we'll end up with way too much "works in testing, > but is a nice CVE" code :-/ We indeed can have both variants of the borrowed/non-borrowed functions. Thanks again for the suggestions
On Tue, 30 Jan 2024 09:34:29 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Jan 26, 2024 at 07:43:29PM +0300, Dmitry Osipenko wrote: > > On 1/26/24 13:18, Boris Brezillon wrote: > > > On Thu, 25 Jan 2024 18:24:04 +0100 > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > >> On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: > > >>> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation > > >>> lock if pages_use_count is non-zero, leveraging from atomicity of the > > >>> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. > > >>> > > >>> Acked-by: Maxime Ripard <mripard@kernel.org> > > >>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > > >>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > > >>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > >>> --- > > >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- > > >>> 1 file changed, 15 insertions(+), 4 deletions(-) > > >>> > > >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > >>> index cacf0f8c42e2..1c032513abf1 100644 > > >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > >>> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) > > >>> } > > >>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); > > >>> > > >>> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > > >>> +{ > > >>> + int ret; > > >> > > >> Just random drive-by comment: a might_lock annotation here might be good, > > >> or people could hit some really interesting bugs that are rather hard to > > >> reproduce ... > > > > > > Actually, being able to acquire a ref in a dma-signalling path on an > > > object we know for sure already has refcount >= 1 (because we previously > > > acquired a ref in a path where dma_resv_lock() was allowed), was the > > > primary reason I suggested moving to this atomic-refcount approach. > > > > > > In the meantime, drm_gpuvm has evolved in a way that allows me to not > > > take the ref in the dma-signalling path (the gpuvm_bo object now holds > > > the ref, and it's acquired/released outside the dma-signalling path). > > > > > > Not saying we shouldn't add this might_lock(), but others might have > > > good reasons to have this function called in a path where locking > > > is not allowed. > > > > For Panthor the might_lock indeed won't be a appropriate, thanks for > > reminding about it. I'll add explanatory comment to the code. > > Hm these kind of tricks feel very dangerous to me. I think it would be > good to split up the two cases into two functions: > > 1. first one does only the atomic_inc and splats if the refcount is zero. > I think something in the name that denotes that we're incrementing a > borrowed pages reference would be good here, so like get_borrowed_pages > (there's not really a naming convention for these in the kernel). > Unfortunately no rust so we can't enforce that you provide the right kind > of borrowed reference at compile time. Yeah, I also considered adding a dedicated function for that use case at some point, instead of abusing get_pages(). Given I no longer need it, we can probably add this might_lock() and defer the addition of this get_borrowed_pages() helper until someone actually needs it. > > 2. second one has the might_lock. > > This way you force callers to think what they're doing and ideally > document where the borrowed reference is from, and ideally document that > in the code. Otherwise we'll end up with way too much "works in testing, > but is a nice CVE" code :-/ Totally agree with you on that point.
On 1/30/24 13:10, Boris Brezillon wrote: > On Tue, 30 Jan 2024 09:34:29 +0100 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Fri, Jan 26, 2024 at 07:43:29PM +0300, Dmitry Osipenko wrote: >>> On 1/26/24 13:18, Boris Brezillon wrote: >>>> On Thu, 25 Jan 2024 18:24:04 +0100 >>>> Daniel Vetter <daniel@ffwll.ch> wrote: >>>> >>>>> On Fri, Jan 05, 2024 at 09:46:03PM +0300, Dmitry Osipenko wrote: >>>>>> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation >>>>>> lock if pages_use_count is non-zero, leveraging from atomicity of the >>>>>> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper. >>>>>> >>>>>> Acked-by: Maxime Ripard <mripard@kernel.org> >>>>>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> >>>>>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> >>>>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++++++++++++++---- >>>>>> 1 file changed, 15 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> index cacf0f8c42e2..1c032513abf1 100644 >>>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); >>>>>> >>>>>> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) >>>>>> +{ >>>>>> + int ret; >>>>> >>>>> Just random drive-by comment: a might_lock annotation here might be good, >>>>> or people could hit some really interesting bugs that are rather hard to >>>>> reproduce ... >>>> >>>> Actually, being able to acquire a ref in a dma-signalling path on an >>>> object we know for sure already has refcount >= 1 (because we previously >>>> acquired a ref in a path where dma_resv_lock() was allowed), was the >>>> primary reason I suggested moving to this atomic-refcount approach. >>>> >>>> In the meantime, drm_gpuvm has evolved in a way that allows me to not >>>> take the ref in the dma-signalling path (the gpuvm_bo object now holds >>>> the ref, and it's acquired/released outside the dma-signalling path). >>>> >>>> Not saying we shouldn't add this might_lock(), but others might have >>>> good reasons to have this function called in a path where locking >>>> is not allowed. >>> >>> For Panthor the might_lock indeed won't be a appropriate, thanks for >>> reminding about it. I'll add explanatory comment to the code. >> >> Hm these kind of tricks feel very dangerous to me. I think it would be >> good to split up the two cases into two functions: >> >> 1. first one does only the atomic_inc and splats if the refcount is zero. >> I think something in the name that denotes that we're incrementing a >> borrowed pages reference would be good here, so like get_borrowed_pages >> (there's not really a naming convention for these in the kernel). >> Unfortunately no rust so we can't enforce that you provide the right kind >> of borrowed reference at compile time. > > Yeah, I also considered adding a dedicated function for that use case > at some point, instead of abusing get_pages(). Given I no longer need > it, we can probably add this might_lock() and defer the addition of this > get_borrowed_pages() helper until someone actually needs it. Ack, I'll add the might_lock() then. Missed previously that you don't need to use get_pages() anymore. Thanks
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index cacf0f8c42e2..1c032513abf1 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem) } EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked); +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) +{ + int ret; + + if (refcount_inc_not_zero(&shmem->pages_use_count)) + return 0; + + dma_resv_lock(shmem->base.resv, NULL); + ret = drm_gem_shmem_get_pages_locked(shmem); + dma_resv_unlock(shmem->base.resv); + + return ret; +} + static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem) { int ret; @@ -609,10 +623,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct return ret; } - dma_resv_lock(shmem->base.resv, NULL); - ret = drm_gem_shmem_get_pages_locked(shmem); - dma_resv_unlock(shmem->base.resv); - + ret = drm_gem_shmem_get_pages(shmem); if (ret) return ret;