* [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
@ 2020-11-01 17:40 ` Boris Brezillon
0 siblings, 0 replies; 6+ 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] 6+ messages in thread* [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
@ 2020-11-01 17:40 ` Boris Brezillon
0 siblings, 0 replies; 6+ 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: dri-devel, Boris Brezillon, stable, Christian Hewitt
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [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
-1 siblings, 0 replies; 6+ 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] 6+ messages in thread* Re: [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
@ 2020-11-02 8:42 ` Steven Price
0 siblings, 0 replies; 6+ 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: dri-devel, stable, Christian Hewitt
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
>
^ permalink raw reply [flat|nested] 6+ 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
-1 siblings, 0 replies; 6+ 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] 6+ messages in thread* Re: [PATCH] drm/panfrost: Fix a deadlock between the shrinker and madvise path
@ 2020-11-03 8:45 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2020-11-03 8:45 UTC (permalink / raw)
To: Steven Price
Cc: Rob Herring, Alyssa Rosenzweig, Robin Murphy, dri-devel, stable,
Christian Hewitt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-03 8:45 UTC | newest]
Thread overview: 6+ 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-01 17:40 ` Boris Brezillon
2020-11-02 8:42 ` Steven Price
2020-11-02 8:42 ` Steven Price
2020-11-03 8:45 ` Boris Brezillon
2020-11-03 8:45 ` Boris Brezillon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.