On 3/11/26 09:47, Khatri, Sunil wrote:
>
> On 11-03-2026 12:43 am, Christian König wrote:
>> Instead of comming up with more sophisticated names for states a VM BO
>> can be in group them by the type of BO first and then by the state.
>>
>> So we end with BO type kernel, shared_resv and individual_resv and then
>> states evicted, moved and idle.
>>
>> Not much functional change, except that evicted_user is moved back
>> together with the other BOs again which makes the handling in
>> amdgpu_vm_validate() a bit more complex. Also fixes a problem with user
>> queues and amdgpu_vm_ready().
> Some typos in commit message.
Can you point them out? I only the the extra "m" in the first sentence of hand.
>> @@ -349,46 +366,22 @@ struct amdgpu_vm {
>> spinlock_t stats_lock;
>> struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
>>
>> - /*
>> - * The following lists contain amdgpu_vm_bo_base objects for either
>> - * PDs, PTs or per VM BOs. The state transits are:
>> - *
>> - * evicted -> relocated (PDs, PTs) or moved (per VM BOs) -> idle
>> - *
>> - * Lists are protected by the root PD dma_resv lock.
>> - */
>> -
>> - /* Per-VM and PT BOs who needs a validation */
>> - struct list_head evicted;
>> -
>> - /* PT BOs which relocated and their parent need an update */
>> - struct list_head relocated;
>> + /* kernel PD/Pts, resv is shared with the root PD */
>> + struct amdgpu_vm_bo_status kernel;
>>
>> - /* per VM BOs moved, but not yet updated in the PT */
>> - struct list_head moved;
>> -
>> - /* All BOs of this VM not currently in the state machine */
>> - struct list_head idle;
>> + /* userspace BOs where the resv object is shared with the root PD */
>> + struct amdgpu_vm_bo_status shared_resv;
>>
>> /*
>> * The following lists contain amdgpu_vm_bo_base objects for BOs which
>> - * have their own dma_resv object and not depend on the root PD. Their
>> - * state transits are:
>> - *
>> - * evicted_user or invalidated -> done
>> + * have their own dma_resv object and not depend on the root PD.
>
> We might want to move the explanation before the individual_resv list for
> clarity. Grouping idea seems great and seems to simply the things to good
> extent. But just for better explanation and more clarity for others and
> documentation purposes if we want can add more details:
>
> kernel: BO's belonging to PD/PT and other BOs which are internal to the
> kernel.
>
> shared_resv BO's belonging to per process but still allocated by the
> driver/kernel for each process, for name can we says per_process
>
> individual_resv : BO's whose memory is allocated by user for completely user
> managed buffers per process (User ptrs), alternate name : user_private or
> something.
Mhm, that description is actually not correct. Those are not necessarily
userptrs but can also be normal BOs shared with other processes or devices.
I will try to come up with some better comments.
>
> Apart from the nitpicks, the change looks great and simplify things.
>
> Reviewed-by: Sunil Khatri <[email protected]>
Thanks,
Christian.
>
> Regards Sunil Khatri
>
>> *
>> * Lists are protected by the invalidated_lock.
>> */
>> spinlock_t invalidated_lock;
>>
>> - /* BOs for user mode queues that need a validation */
>> - struct list_head evicted_user;
>> -
>> - /* regular invalidated BOs, but not yet updated in the PT */
>> - struct list_head invalidated;
>> -
>> - /* BOs which are invalidated, has been updated in the PTs */
>> - struct list_head done;
>> + /* Userspace BOs with individual resv object */
>> + struct amdgpu_vm_bo_status individual_resv;
>>
>> /*
>> * This list contains amdgpu_bo_va_mapping objects which have been freed
