dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
@ 2020-11-01 17:40 Boris Brezillon
  2020-11-02  8:42 ` Steven Price
  0 siblings, 1 reply; 3+ messages in thread
From: Boris Brezillon @ 2020-11-01 17:40 UTC (permalink / raw)
  To: Rob Herring, Tomeu Vizoso, Alyssa Rosenzweig, Steven Price, Robin Murphy
  Cc: Boris Brezillon, Christian Hewitt, stable, dri-devel

panfrost_ioctl_madvise() and panfrost_gem_purge() acquire the mappings
and shmem locks in different orders, thus leading to a potential
the mappings lock first.

Fixes: bdefca2d8dc0 ("drm/panfrost: Add the panfrost_gem_mapping concept")
Cc: <stable@vger.kernel.org>
Cc: Christian Hewitt <christianshewitt@gmail.com>
Reported-by: Christian Hewitt <christianshewitt@gmail.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_gem.c          |  4 +---
 drivers/gpu/drm/panfrost/panfrost_gem.h          |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 14 +++++++++++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index cdf1a8754eba..0c0243eaee81 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -105,14 +105,12 @@ void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping)
 	kref_put(&mapping->refcount, panfrost_gem_mapping_release);
 }
 
-void panfrost_gem_teardown_mappings(struct panfrost_gem_object *bo)
+void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo)
 {
 	struct panfrost_gem_mapping *mapping;
 
-	mutex_lock(&bo->mappings.lock);
 	list_for_each_entry(mapping, &bo->mappings.list, node)
 		panfrost_gem_teardown_mapping(mapping);
-	mutex_unlock(&bo->mappings.lock);
 }
 
 int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
index b3517ff9630c..8088d5fd8480 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -82,7 +82,7 @@ struct panfrost_gem_mapping *
 panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
 			 struct panfrost_file_priv *priv);
 void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
-void panfrost_gem_teardown_mappings(struct panfrost_gem_object *bo);
+void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
 
 void panfrost_gem_shrinker_init(struct drm_device *dev);
 void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
index 288e46c40673..1b9f68d8e9aa 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
@@ -40,18 +40,26 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj)
 {
 	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
 	struct panfrost_gem_object *bo = to_panfrost_bo(obj);
+	bool ret = false;
 
 	if (atomic_read(&bo->gpu_usecount))
 		return false;
 
-	if (!mutex_trylock(&shmem->pages_lock))
+	if (!mutex_trylock(&bo->mappings.lock))
 		return false;
 
-	panfrost_gem_teardown_mappings(bo);
+	if (!mutex_trylock(&shmem->pages_lock))
+		goto unlock_mappings;
+
+	panfrost_gem_teardown_mappings_locked(bo);
 	drm_gem_shmem_purge_locked(obj);
+	ret = true;
 
 	mutex_unlock(&shmem->pages_lock);
-	return true;
+
+unlock_mappings:
+	mutex_unlock(&bo->mappings.lock);
+	return ret;
 }
 
 static unsigned long
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
  2020-11-01 17:40 [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path Boris Brezillon
@ 2020-11-02  8:42 ` Steven Price
  2020-11-03  8:45   ` Boris Brezillon
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Price @ 2020-11-02  8:42 UTC (permalink / raw)
  To: Boris Brezillon, Rob Herring, Tomeu Vizoso, Alyssa Rosenzweig,
	Robin Murphy
  Cc: Christian Hewitt, stable, dri-devel

On 01/11/2020 17:40, Boris Brezillon wrote:
> panfrost_ioctl_madvise() and panfrost_gem_purge() acquire the mappings
> and shmem locks in different orders, thus leading to a potential
> the mappings lock first.
> 
> Fixes: bdefca2d8dc0 ("drm/panfrost: Add the panfrost_gem_mapping concept")
> Cc: <stable@vger.kernel.org>
> Cc: Christian Hewitt <christianshewitt@gmail.com>
> Reported-by: Christian Hewitt <christianshewitt@gmail.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>   drivers/gpu/drm/panfrost/panfrost_gem.c          |  4 +---
>   drivers/gpu/drm/panfrost/panfrost_gem.h          |  2 +-
>   drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 14 +++++++++++---
>   3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index cdf1a8754eba..0c0243eaee81 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -105,14 +105,12 @@ void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping)
>   	kref_put(&mapping->refcount, panfrost_gem_mapping_release);
>   }
>   
> -void panfrost_gem_teardown_mappings(struct panfrost_gem_object *bo)
> +void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo)
>   {
>   	struct panfrost_gem_mapping *mapping;
>   
> -	mutex_lock(&bo->mappings.lock);
>   	list_for_each_entry(mapping, &bo->mappings.list, node)
>   		panfrost_gem_teardown_mapping(mapping);
> -	mutex_unlock(&bo->mappings.lock);
>   }
>   
>   int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv)
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> index b3517ff9630c..8088d5fd8480 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> @@ -82,7 +82,7 @@ struct panfrost_gem_mapping *
>   panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
>   			 struct panfrost_file_priv *priv);
>   void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
> -void panfrost_gem_teardown_mappings(struct panfrost_gem_object *bo);
> +void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
>   
>   void panfrost_gem_shrinker_init(struct drm_device *dev);
>   void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> index 288e46c40673..1b9f68d8e9aa 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> @@ -40,18 +40,26 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj)
>   {
>   	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>   	struct panfrost_gem_object *bo = to_panfrost_bo(obj);
> +	bool ret = false;
>   
>   	if (atomic_read(&bo->gpu_usecount))
>   		return false;
>   
> -	if (!mutex_trylock(&shmem->pages_lock))
> +	if (!mutex_trylock(&bo->mappings.lock))
>   		return false;
>   
> -	panfrost_gem_teardown_mappings(bo);
> +	if (!mutex_trylock(&shmem->pages_lock))
> +		goto unlock_mappings;
> +
> +	panfrost_gem_teardown_mappings_locked(bo);
>   	drm_gem_shmem_purge_locked(obj);
> +	ret = true;
>   
>   	mutex_unlock(&shmem->pages_lock);
> -	return true;
> +
> +unlock_mappings:
> +	mutex_unlock(&bo->mappings.lock);
> +	return ret;
>   }
>   
>   static unsigned long
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
  2020-11-02  8:42 ` Steven Price
@ 2020-11-03  8:45   ` Boris Brezillon
  0 siblings, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2020-11-03  8:45 UTC (permalink / raw)
  To: Steven Price
  Cc: dri-devel, Christian Hewitt, stable, Rob Herring,
	Alyssa Rosenzweig, Robin Murphy

On Mon, 2 Nov 2020 08:42:49 +0000
Steven Price <steven.price@arm.com> wrote:

> On 01/11/2020 17:40, Boris Brezillon wrote:
> > panfrost_ioctl_madvise() and panfrost_gem_purge() acquire the mappings
> > and shmem locks in different orders, thus leading to a potential
> > the mappings lock first.
> > 
> > Fixes: bdefca2d8dc0 ("drm/panfrost: Add the panfrost_gem_mapping concept")
> > Cc: <stable@vger.kernel.org>
> > Cc: Christian Hewitt <christianshewitt@gmail.com>
> > Reported-by: Christian Hewitt <christianshewitt@gmail.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>  
> 
> Reviewed-by: Steven Price <steven.price@arm.com>

Queued to drm-misc-fixes.

Thanks,

Boris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-03  8:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-01 17:40 [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path Boris Brezillon
2020-11-02  8:42 ` Steven Price
2020-11-03  8:45   ` Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).