drm/msm: skip re-emitting IBs for unusable VMs

[PATCH] drm/msm: skip re-emitting IBs for unusable VMs

Posted by Antonino Maniscalco 1 month, 3 weeks ago

When a VM is marked as an usuable we disallow new submissions from it,
however submissions that where already scheduled on the ring would still
be re-sent.

Since this can lead to further hangs, avoid emitting the actual IBs.

Fixes: 6a4d287a1ae6 ("drm/msm: Mark VM as unusable on GPU hangs")
Signed-off-by: Antonino Maniscalco <antomani103@gmail.com>
---
 drivers/gpu/drm/msm/msm_gpu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index c317b25a8162edba0d594f61427eac4440871b73..e6cd85c810bd2314c8bba53644a622464713b7f2 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -553,8 +553,15 @@ static void recover_worker(struct kthread_work *work)
 			unsigned long flags;
 
 			spin_lock_irqsave(&ring->submit_lock, flags);
-			list_for_each_entry(submit, &ring->submits, node)
+			list_for_each_entry(submit, &ring->submits, node) {
+				/*
+				 * If the submit uses an unusable vm make sure
+				 * we don't actually run it
+				 */
+				if (to_msm_vm(submit->vm)->unusable)
+					submit->nr_cmds = 0;
 				gpu->funcs->submit(gpu, submit);
+			}
 			spin_unlock_irqrestore(&ring->submit_lock, flags);
 		}
 	}

---
base-commit: 8290d37ad2b087bbcfe65fa5bcaf260e184b250a
change-id: 20250813-unusable_fix_b4-10bde6f3b756

Best regards,
-- 
Antonino Maniscalco <antomani103@gmail.com>

Re: [PATCH] drm/msm: skip re-emitting IBs for unusable VMs

Posted by Akhil P Oommen 1 month, 2 weeks ago

On 8/13/2025 6:34 PM, Antonino Maniscalco wrote:
> When a VM is marked as an usuable we disallow new submissions from it,
> however submissions that where already scheduled on the ring would still
> be re-sent.
> 
> Since this can lead to further hangs, avoid emitting the actual IBs.
> 
> Fixes: 6a4d287a1ae6 ("drm/msm: Mark VM as unusable on GPU hangs")
> Signed-off-by: Antonino Maniscalco <antomani103@gmail.com>
> ---
>  drivers/gpu/drm/msm/msm_gpu.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index c317b25a8162edba0d594f61427eac4440871b73..e6cd85c810bd2314c8bba53644a622464713b7f2 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -553,8 +553,15 @@ static void recover_worker(struct kthread_work *work)
>  			unsigned long flags;
>  
>  			spin_lock_irqsave(&ring->submit_lock, flags);
> -			list_for_each_entry(submit, &ring->submits, node)
> +			list_for_each_entry(submit, &ring->submits, node) {
> +				/*
> +				 * If the submit uses an unusable vm make sure
> +				 * we don't actually run it
> +				 */
> +				if (to_msm_vm(submit->vm)->unusable)
> +					submit->nr_cmds = 0;

Just curious, why not just retire this submit here?

-Akhil

>  				gpu->funcs->submit(gpu, submit);
> +			}
>  			spin_unlock_irqrestore(&ring->submit_lock, flags);
>  		}
>  	}
> 
> ---
> base-commit: 8290d37ad2b087bbcfe65fa5bcaf260e184b250a
> change-id: 20250813-unusable_fix_b4-10bde6f3b756
> 
> Best regards,

Re: [PATCH] drm/msm: skip re-emitting IBs for unusable VMs

Posted by Rob Clark 1 month, 2 weeks ago

On Mon, Aug 18, 2025 at 5:10 AM Akhil P Oommen <akhilpo@oss.qualcomm.com> wrote:
>
> On 8/13/2025 6:34 PM, Antonino Maniscalco wrote:
> > When a VM is marked as an usuable we disallow new submissions from it,
> > however submissions that where already scheduled on the ring would still
> > be re-sent.
> >
> > Since this can lead to further hangs, avoid emitting the actual IBs.
> >
> > Fixes: 6a4d287a1ae6 ("drm/msm: Mark VM as unusable on GPU hangs")
> > Signed-off-by: Antonino Maniscalco <antomani103@gmail.com>
> > ---
> >  drivers/gpu/drm/msm/msm_gpu.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> > index c317b25a8162edba0d594f61427eac4440871b73..e6cd85c810bd2314c8bba53644a622464713b7f2 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu.c
> > +++ b/drivers/gpu/drm/msm/msm_gpu.c
> > @@ -553,8 +553,15 @@ static void recover_worker(struct kthread_work *work)
> >                       unsigned long flags;
> >
> >                       spin_lock_irqsave(&ring->submit_lock, flags);
> > -                     list_for_each_entry(submit, &ring->submits, node)
> > +                     list_for_each_entry(submit, &ring->submits, node) {
> > +                             /*
> > +                              * If the submit uses an unusable vm make sure
> > +                              * we don't actually run it
> > +                              */
> > +                             if (to_msm_vm(submit->vm)->unusable)
> > +                                     submit->nr_cmds = 0;
>
> Just curious, why not just retire this submit here?

Because then you'd end up with submits retiring out of order (ie.
fences on the same timeline signaling out of order)

BR,
-R

> -Akhil
>
> >                               gpu->funcs->submit(gpu, submit);
> > +                     }
> >                       spin_unlock_irqrestore(&ring->submit_lock, flags);
> >               }
> >       }
> >
> > ---
> > base-commit: 8290d37ad2b087bbcfe65fa5bcaf260e184b250a
> > change-id: 20250813-unusable_fix_b4-10bde6f3b756
> >
> > Best regards,
>

Re: [PATCH] drm/msm: skip re-emitting IBs for unusable VMs

Posted by Akhil P Oommen 1 month, 2 weeks ago

On 8/18/2025 6:48 PM, Rob Clark wrote:
> On Mon, Aug 18, 2025 at 5:10 AM Akhil P Oommen <akhilpo@oss.qualcomm.com> wrote:
>>
>> On 8/13/2025 6:34 PM, Antonino Maniscalco wrote:
>>> When a VM is marked as an usuable we disallow new submissions from it,
>>> however submissions that where already scheduled on the ring would still
>>> be re-sent.
>>>
>>> Since this can lead to further hangs, avoid emitting the actual IBs.
>>>
>>> Fixes: 6a4d287a1ae6 ("drm/msm: Mark VM as unusable on GPU hangs")
>>> Signed-off-by: Antonino Maniscalco <antomani103@gmail.com>
>>> ---
>>>  drivers/gpu/drm/msm/msm_gpu.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
>>> index c317b25a8162edba0d594f61427eac4440871b73..e6cd85c810bd2314c8bba53644a622464713b7f2 100644
>>> --- a/drivers/gpu/drm/msm/msm_gpu.c
>>> +++ b/drivers/gpu/drm/msm/msm_gpu.c
>>> @@ -553,8 +553,15 @@ static void recover_worker(struct kthread_work *work)
>>>                       unsigned long flags;
>>>
>>>                       spin_lock_irqsave(&ring->submit_lock, flags);
>>> -                     list_for_each_entry(submit, &ring->submits, node)
>>> +                     list_for_each_entry(submit, &ring->submits, node) {
>>> +                             /*
>>> +                              * If the submit uses an unusable vm make sure
>>> +                              * we don't actually run it
>>> +                              */
>>> +                             if (to_msm_vm(submit->vm)->unusable)
>>> +                                     submit->nr_cmds = 0;
>>
>> Just curious, why not just retire this submit here?
> 
> Because then you'd end up with submits retiring out of order (ie.
> fences on the same timeline signaling out of order)

Yeah, that makes sense.

Reviewed-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>

-Akhil

> 
> BR,
> -R
> 
>> -Akhil
>>
>>>                               gpu->funcs->submit(gpu, submit);
>>> +                     }
>>>                       spin_unlock_irqrestore(&ring->submit_lock, flags);
>>>               }
>>>       }
>>>
>>> ---
>>> base-commit: 8290d37ad2b087bbcfe65fa5bcaf260e184b250a
>>> change-id: 20250813-unusable_fix_b4-10bde6f3b756
>>>
>>> Best regards,
>>