On 5/14/26 14:42, Prike Liang wrote:
> If the driver only preempts queues, there can still be inflight waves,
> pending dispatch state, or resume/redispatch possibility tied to the
> same queue. Then the VM/TTM side may proceed to move/unmap queue related
> BOs during evicting the queue while shader TCP clients still need to
> access them.
> 
> So for eviction, unmap is safer because it makes the queue nonrunnable
> before memory backing is invalidated. Meanwhile, for a idle queue it's
> more sutiable for unmapping it rather preempt and unmapping also safe more
> processing time than preempt.
> 
> Signed-off-by: Prike Liang <[email protected]>

I was already wondering when we switched to preempt if that is correct or not.

This patch here pretty much confirms that it was not correct. I need to discuss 
with Alex what to do here, but for now the patch is clearly a bug fix:

Reviewed-by: Christian König <[email protected]>

Thanks,
Christian.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 2e3edb6dd506..a63dfdfa4195 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1048,7 +1048,7 @@ amdgpu_userq_restore_all(struct amdgpu_userq_mgr 
> *uq_mgr)
>                       continue;
>               }
>  
> -             r = amdgpu_userq_restore_helper(queue);
> +             r = amdgpu_userq_map_helper(queue);
>               if (r)
>                       ret = r;
>  
> @@ -1285,7 +1285,7 @@ amdgpu_userq_evict_all(struct amdgpu_userq_mgr *uq_mgr)
>  
>       /* Try to unmap all the queues in this process ctx */
>       xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
> -             r = amdgpu_userq_preempt_helper(queue);
> +             r = amdgpu_userq_unmap_helper(queue);
>               if (r)
>                       ret = r;
>       }

Reply via email to