Re: [PATCH 1/2] drm/gpuvm: add deferred vm_bo cleanup

[Danilo Krummrich <dakr@kernel.org>]

On 9/9/25 1:24 PM, Alice Ryhl wrote:
> On Tue, Sep 9, 2025 at 1:11 PM Thomas Hellström
> <thomas.hellstrom@linux.intel.com> wrote:
>>
>> On Tue, 2025-09-09 at 12:47 +0200, Danilo Krummrich wrote:
>>> On 9/9/25 12:39 PM, Thomas Hellström wrote:
>>>> On 9/8/25 14:20, Danilo Krummrich wrote:
>>>>> On 9/8/25 2:11 PM, Boris Brezillon wrote:
>>>>>> On Mon, 08 Sep 2025 13:11:32 +0200
>>>>>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>>>>>> No, drivers can't iterate the evict/extobj lists directly; or
>>>>>>> at least this is
>>>>>>> not intended by GPUVM's API and if drivers do so, this is
>>>>>>> considered peeking
>>>>>>> into GPUVM internals, so drivers are on their own anyways.
>>>>>>>
>>>>>>> Iterators, such as for_each_vm_bo_in_list() are not exposed
>>>>>>> to drivers.
>>>>>> Okay, that's a good thing. I thought Xe was doing some funky
>>>>>> stuff with
>>>>>> the list...
>>>>> Maybe, I don't know. If they do so, the should send patches
>>>>> adding the
>>>>> corresponding iterators and provide a rationale why drivers need
>>>>> to access those
>>>>> lists directly and why we can't provide an API that handles the
>>>>> overall
>>>>> use-case, such as drm_gpuvm_prepare_objects(), etc.
>>>>
>>>> We're using the drm_gpuvm_*for_each* macros in drm_gpuvm.h,
>>>> assuming from name
>>>> and docs they are driver api.
>>>>
>>>> Also the drm_gem_for_each_gpuvm_bo(), although this usage could
>>>> easily be
>>>> converted to a helper.
>>>
>>> We were talking about the extobj/evict lists, the ones you mention
>>> are fine of
>>> course. :)
>>>
>>
>> Hmm. Now on closer inspection it looks like we're checking for evict
>> list empty, It looks like rebinding after validation may in theory
>> evict some bos to system memory and then we'd rerun the validation step
>> if the evict list was not empty.
>>
>> We could of course add a helper for that or if there are better
>> suggestions to handle that situation, that'd be fine as well.
> 
> I don't think evict list empty means that there are no evicted GEMs.
> It's possible for an extobj to be missing from the evict list in some
> scenarios. That's why drm_gpuvm_prepare_objects_locked() checks
> evicted on the extobj list to ensure that the evicted list is
> up-to-date when you call into drm_gpuvm_validate_locked().

Indeed, though I would expect that Xe considers that? It was Thomas who proposed
the logic you describe here back then IIRC. :)