All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/gem: Drop early returns in close/purge vma
@ 2022-06-10 17:20 ` Rob Clark
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2022-06-10 17:20 UTC (permalink / raw)
  To: dri-devel
  Cc: freedreno, linux-arm-msm, Rob Clark, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, David Airlie, Daniel Vetter,
	open list

From: Rob Clark <robdclark@chromium.org>

Keep the warn, but drop the early return.  If we do manage to hit this
sort of issue, skipping the cleanup just makes things worse (dangling
drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
that happens if we tear down a mapping the GPU is accessing is that we
get GPU iova faults, but otherwise the world keeps spinning.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 3c1dc9241831..c471aebcdbab 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
 	unsigned size = vma->node.size;
 
 	/* Print a message if we try to purge a vma in use */
-	if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
-		return;
+	GEM_WARN_ON(msm_gem_vma_inuse(vma));
 
 	/* Don't do anything if the memory isn't mapped */
 	if (!vma->mapped)
@@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
 void msm_gem_close_vma(struct msm_gem_address_space *aspace,
 		struct msm_gem_vma *vma)
 {
-	if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
-		return;
+	GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
 
 	spin_lock(&aspace->lock);
 	if (vma->iova)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] drm/msm/gem: Drop early returns in close/purge vma
@ 2022-06-10 17:20 ` Rob Clark
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2022-06-10 17:20 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, David Airlie, linux-arm-msm, Abhinav Kumar, open list,
	Sean Paul, Dmitry Baryshkov, freedreno

From: Rob Clark <robdclark@chromium.org>

Keep the warn, but drop the early return.  If we do manage to hit this
sort of issue, skipping the cleanup just makes things worse (dangling
drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
that happens if we tear down a mapping the GPU is accessing is that we
get GPU iova faults, but otherwise the world keeps spinning.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 3c1dc9241831..c471aebcdbab 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
 	unsigned size = vma->node.size;
 
 	/* Print a message if we try to purge a vma in use */
-	if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
-		return;
+	GEM_WARN_ON(msm_gem_vma_inuse(vma));
 
 	/* Don't do anything if the memory isn't mapped */
 	if (!vma->mapped)
@@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
 void msm_gem_close_vma(struct msm_gem_address_space *aspace,
 		struct msm_gem_vma *vma)
 {
-	if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
-		return;
+	GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
 
 	spin_lock(&aspace->lock);
 	if (vma->iova)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
  2022-06-10 17:20 ` Rob Clark
@ 2022-06-11 18:16   ` Steev Klimaszewski
  -1 siblings, 0 replies; 6+ messages in thread
From: Steev Klimaszewski @ 2022-06-11 18:16 UTC (permalink / raw)
  To: Rob Clark, dri-devel
  Cc: freedreno, linux-arm-msm, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, David Airlie, Daniel Vetter,
	open list

Hi Rob,

On 6/10/22 12:20 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> Keep the warn, but drop the early return.  If we do manage to hit this
> sort of issue, skipping the cleanup just makes things worse (dangling
> drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
> that happens if we tear down a mapping the GPU is accessing is that we
> get GPU iova faults, but otherwise the world keeps spinning.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 3c1dc9241831..c471aebcdbab 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
>   	unsigned size = vma->node.size;
>   
>   	/* Print a message if we try to purge a vma in use */
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma));
>   
>   	/* Don't do anything if the memory isn't mapped */
>   	if (!vma->mapped)
> @@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
>   void msm_gem_close_vma(struct msm_gem_address_space *aspace,
>   		struct msm_gem_vma *vma)
>   {
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
>   
>   	spin_lock(&aspace->lock);
>   	if (vma->iova)

I've seen the splat on the Lenovo Yoga C630 here, and have tested this 
patch, and as described, the splat still happens, but the system is 
still able to be used.

Tested-by: Steev Klimaszewski <steev@kali.org>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
@ 2022-06-11 18:16   ` Steev Klimaszewski
  0 siblings, 0 replies; 6+ messages in thread
From: Steev Klimaszewski @ 2022-06-11 18:16 UTC (permalink / raw)
  To: Rob Clark, dri-devel
  Cc: Rob Clark, freedreno, David Airlie, linux-arm-msm, Abhinav Kumar,
	open list, Dmitry Baryshkov, Sean Paul

Hi Rob,

On 6/10/22 12:20 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> Keep the warn, but drop the early return.  If we do manage to hit this
> sort of issue, skipping the cleanup just makes things worse (dangling
> drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
> that happens if we tear down a mapping the GPU is accessing is that we
> get GPU iova faults, but otherwise the world keeps spinning.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 3c1dc9241831..c471aebcdbab 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
>   	unsigned size = vma->node.size;
>   
>   	/* Print a message if we try to purge a vma in use */
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma));
>   
>   	/* Don't do anything if the memory isn't mapped */
>   	if (!vma->mapped)
> @@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
>   void msm_gem_close_vma(struct msm_gem_address_space *aspace,
>   		struct msm_gem_vma *vma)
>   {
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
>   
>   	spin_lock(&aspace->lock);
>   	if (vma->iova)

I've seen the splat on the Lenovo Yoga C630 here, and have tested this 
patch, and as described, the splat still happens, but the system is 
still able to be used.

Tested-by: Steev Klimaszewski <steev@kali.org>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
  2022-06-11 18:16   ` Steev Klimaszewski
@ 2022-06-15 14:59     ` Rob Clark
  -1 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2022-06-15 14:59 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: dri-devel, freedreno, linux-arm-msm, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, David Airlie, Daniel Vetter,
	open list

On Sat, Jun 11, 2022 at 11:16 AM Steev Klimaszewski <steev@kali.org> wrote:
>
> Hi Rob,
>
> On 6/10/22 12:20 PM, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Keep the warn, but drop the early return.  If we do manage to hit this
> > sort of issue, skipping the cleanup just makes things worse (dangling
> > drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
> > that happens if we tear down a mapping the GPU is accessing is that we
> > get GPU iova faults, but otherwise the world keeps spinning.
> >

forgot this initially:

Reported-by: Steev Klimaszewski <steev@kali.org>

> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >   drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > index 3c1dc9241831..c471aebcdbab 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > @@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
> >       unsigned size = vma->node.size;
> >
> >       /* Print a message if we try to purge a vma in use */
> > -     if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
> > -             return;
> > +     GEM_WARN_ON(msm_gem_vma_inuse(vma));
> >
> >       /* Don't do anything if the memory isn't mapped */
> >       if (!vma->mapped)
> > @@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
> >   void msm_gem_close_vma(struct msm_gem_address_space *aspace,
> >               struct msm_gem_vma *vma)
> >   {
> > -     if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
> > -             return;
> > +     GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
> >
> >       spin_lock(&aspace->lock);
> >       if (vma->iova)
>
> I've seen the splat on the Lenovo Yoga C630 here, and have tested this
> patch, and as described, the splat still happens, but the system is
> still able to be used.
>
> Tested-by: Steev Klimaszewski <steev@kali.org>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
@ 2022-06-15 14:59     ` Rob Clark
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2022-06-15 14:59 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: Rob Clark, David Airlie, linux-arm-msm, Abhinav Kumar, dri-devel,
	open list, Sean Paul, Dmitry Baryshkov, freedreno

On Sat, Jun 11, 2022 at 11:16 AM Steev Klimaszewski <steev@kali.org> wrote:
>
> Hi Rob,
>
> On 6/10/22 12:20 PM, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Keep the warn, but drop the early return.  If we do manage to hit this
> > sort of issue, skipping the cleanup just makes things worse (dangling
> > drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
> > that happens if we tear down a mapping the GPU is accessing is that we
> > get GPU iova faults, but otherwise the world keeps spinning.
> >

forgot this initially:

Reported-by: Steev Klimaszewski <steev@kali.org>

> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >   drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > index 3c1dc9241831..c471aebcdbab 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > @@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
> >       unsigned size = vma->node.size;
> >
> >       /* Print a message if we try to purge a vma in use */
> > -     if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
> > -             return;
> > +     GEM_WARN_ON(msm_gem_vma_inuse(vma));
> >
> >       /* Don't do anything if the memory isn't mapped */
> >       if (!vma->mapped)
> > @@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
> >   void msm_gem_close_vma(struct msm_gem_address_space *aspace,
> >               struct msm_gem_vma *vma)
> >   {
> > -     if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
> > -             return;
> > +     GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
> >
> >       spin_lock(&aspace->lock);
> >       if (vma->iova)
>
> I've seen the splat on the Lenovo Yoga C630 here, and have tested this
> patch, and as described, the splat still happens, but the system is
> still able to be used.
>
> Tested-by: Steev Klimaszewski <steev@kali.org>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-15 14:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 17:20 [PATCH] drm/msm/gem: Drop early returns in close/purge vma Rob Clark
2022-06-10 17:20 ` Rob Clark
2022-06-11 18:16 ` Steev Klimaszewski
2022-06-11 18:16   ` Steev Klimaszewski
2022-06-15 14:59   ` Rob Clark
2022-06-15 14:59     ` Rob Clark

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.