Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
On 1/30/24 13:10, Boris Brezillon wrote:
> On Tue, 30 Jan 2024 09:34:29 +0100
> 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 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
>> Reviewed-by: Boris Brezillon
>> Suggested-by: Boris Brezillon
>> Signed-off-by: Dmitry Osipenko
>> ---
>> 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
--
Best regards,
Dmitry
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
On Tue, 30 Jan 2024 09:34:29 +0100
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 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
> > >>> Reviewed-by: Boris Brezillon
> > >>> Suggested-by: Boris Brezillon
> > >>> Signed-off-by: Dmitry Osipenko
> > >>> ---
> > >>> 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.
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
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 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
> Reviewed-by: Boris Brezillon
> Suggested-by: Boris Brezillon
> Signed-off-by: Dmitry Osipenko
> ---
> 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
--
Best regards,
Dmitry
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
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 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
> >>> Reviewed-by: Boris Brezillon
> >>> Suggested-by: Boris Brezillon
> >>> Signed-off-by: Dmitry Osipenko
> >>> ---
> >>> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
On 1/26/24 13:18, Boris Brezillon wrote:
> On Thu, 25 Jan 2024 18:24:04 +0100
> 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
>>> Reviewed-by: Boris Brezillon
>>> Suggested-by: Boris Brezillon
>>> Signed-off-by: Dmitry Osipenko
>>> ---
>>> 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.
--
Best regards,
Dmitry
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
On Thu, 25 Jan 2024 18:24:04 +0100
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
> > Reviewed-by: Boris Brezillon
> > Suggested-by: Boris Brezillon
> > Signed-off-by: Dmitry Osipenko
> > ---
> > 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.
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
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
>> Reviewed-by: Boris Brezillon
>> Suggested-by: Boris Brezillon
>> Signed-off-by: Dmitry Osipenko
>> ---
>> 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!
--
Best regards,
Dmitry
Re: [PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
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
> Reviewed-by: Boris Brezillon
> Suggested-by: Boris Brezillon
> Signed-off-by: Dmitry Osipenko
> ---
> 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(>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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v19 09/30] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()
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
Reviewed-by: Boris Brezillon
Suggested-by: Boris Brezillon
Signed-off-by: Dmitry Osipenko
---
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;
+
+ if (refcount_inc_not_zero(>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
