On 03/02/2026 08:09, Boris Brezillon wrote:
> On Mon, 2 Feb 2026 20:08:30 +0000
> Akash Goel <[email protected]> wrote:
> 
>>>> +bool panthor_gem_try_evict(struct drm_gem_object *obj,
>>>> +                     struct ww_acquire_ctx *ticket)  
>>>
>>> I think this could be static - I don't see any reference outside this
>>> file (other than the header).
> 
> Will do.
> 
>>>   
>>>> +{
>>>> +  /*
>>>> +   * Track last locked entry for unwinding locks in error and
>>>> +   * success paths
>>>> +   */
>>>> +  struct panthor_gem_object *bo = to_panthor_bo(obj);
>>>> +  struct drm_gpuvm_bo *vm_bo, *last_locked = NULL;
>>>> +  enum panthor_gem_reclaim_state old_state;
>>>> +  int ret = 0;
>>>> +
>>>> +  /* To avoid potential lock ordering issue between bo_gpuva and
>>>> +   * mapping->i_mmap_rwsem, unmap the pages from CPU side before
>>>> +   * acquring the bo_gpuva lock. As the bo_resv lock is held, CPU
>>>> +   * page fault handler won't be able to map in the pages whilst
>>>> +   * eviction is in progress.
>>>> +   */
>>>> +  drm_vma_node_unmap(&bo->base.vma_node, 
>>>> bo->base.dev->anon_inode->i_mapping);  
>>>
>>> There might be an issue here - drm_gem_lru_scan() will have taken the
>>> resv lock. drm_vma_node_unmap() could cause a callback to
>>> panthor_vm_close(). If that ends up being the last reference to
>>> bo->cmap.mmap_count then we'll deadlock attempting to aquire the resv
>>> lock again.  
>>
>> Actually drm_vma_node_unmap() would just invalidate the CPU PTEs.
>> The CPU mapping won't be removed and so panthor_vm_close() won't get called.
> 
> Yep, that's also my understanding of drm_vma_node_unmap(): it kills the
> relevant PTEs in the user VM, but leave the VMA active, so next time
> there's an access, the fault handler will be called.
> 
>>
>>>
>>> I not 100% on that, and sadly it seems my test setup has died so I can't
>>> test that out today.
>>>  
>>
>> We have tests that tries to trigger an evicition for a CPU mapped BO and 
>> so far we didn't see a deadlock problem.
> 
> Actually, that's one of the very few tests I have in my igt branch [1],
> and it was passing fine.
> 
> [1]https://gitlab.freedesktop.org/bbrezillon/igt-gpu-tools/-/commit/fc76934a5579767d2aabe787d40e38a17c3f4ea4#67d3c5d7df01192b03c20b43ad33249c663a95f5_80_97

Cool - I was going to test this out, but I was working at home and my
machine in the office decided to lose its USB devices so I couldn't get
my board working. The joys of working in two different locations.

Anyway, with that resolved, it looks good to me. So with that minor
change to panthor_gem_try_evict() you have my:

Reviewed-by: Steven Price <[email protected]>

Thanks,
Steve

Reply via email to