diff mbox series

[1/2] drm/etnaviv: fix deadlock in GPU coredump

Message ID 20191016142716.10168-1-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/etnaviv: fix deadlock in GPU coredump | expand

Commit Message

Lucas Stach Oct. 16, 2019, 2:27 p.m. UTC
The GPU coredump function violates the locking order by holding the MMU
context lock while trying to acquire the etnaviv_gem_object lock. This
results in a possible ABBA deadlock with other codepaths which follow
the established locking order.
Fortunately this is easy to fix by dropping the MMU context lock
earlier, as the BO dumping doesn't need the MMU context to be stable.
The only thing the BO dumping cares about are the BO mappings, which
are stable across the lifetime of the job.

Fixes: 27b67278e007 (drm/etnaviv: rework MMU handling)
[ Not really the first bad commit, but the one where this fix applies
  cleanly. Stable kernels need a manual backport. ]
Reported-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_dump.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christian Gmeiner Oct. 25, 2019, 10:42 a.m. UTC | #1
Am Mi., 16. Okt. 2019 um 16:27 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>:
>
> The GPU coredump function violates the locking order by holding the MMU
> context lock while trying to acquire the etnaviv_gem_object lock. This
> results in a possible ABBA deadlock with other codepaths which follow
> the established locking order.
> Fortunately this is easy to fix by dropping the MMU context lock
> earlier, as the BO dumping doesn't need the MMU context to be stable.
> The only thing the BO dumping cares about are the BO mappings, which
> are stable across the lifetime of the job.
>
> Fixes: 27b67278e007 (drm/etnaviv: rework MMU handling)
> [ Not really the first bad commit, but the one where this fix applies
>   cleanly. Stable kernels need a manual backport. ]
> Reported-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Tested-by: Christian Gmeiner <christian.gmeiner@gmail.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_dump.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> index 698db540972c..648cf0207309 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> @@ -180,6 +180,8 @@ void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
>                               etnaviv_cmdbuf_get_va(&submit->cmdbuf,
>                                         &gpu->mmu_context->cmdbuf_mapping));
>
> +       mutex_unlock(&gpu->mmu_context->lock);
> +
>         /* Reserve space for the bomap */
>         if (n_bomap_pages) {
>                 bomap_start = bomap = iter.data;
> @@ -221,8 +223,6 @@ void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
>                                          obj->base.size);
>         }
>
> -       mutex_unlock(&gpu->mmu_context->lock);
> -
>         etnaviv_core_dump_header(&iter, ETDUMP_BUF_END, iter.data);
>
>         dev_coredumpv(gpu->dev, iter.start, iter.data - iter.start, GFP_KERNEL);
> --
> 2.20.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
index 698db540972c..648cf0207309 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
@@ -180,6 +180,8 @@  void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
 			      etnaviv_cmdbuf_get_va(&submit->cmdbuf,
 					&gpu->mmu_context->cmdbuf_mapping));
 
+	mutex_unlock(&gpu->mmu_context->lock);
+
 	/* Reserve space for the bomap */
 	if (n_bomap_pages) {
 		bomap_start = bomap = iter.data;
@@ -221,8 +223,6 @@  void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
 					 obj->base.size);
 	}
 
-	mutex_unlock(&gpu->mmu_context->lock);
-
 	etnaviv_core_dump_header(&iter, ETDUMP_BUF_END, iter.data);
 
 	dev_coredumpv(gpu->dev, iter.start, iter.data - iter.start, GFP_KERNEL);