On Sat, 9 Sep 2023 17:31:13 +0200
Danilo Krummrich <d...@redhat.com> wrote:
> +/**
> + * get_next_vm_bo_from_list() - get the next vm_bo element
> + * @__gpuvm: The GPU VM
> + * @__list_name: The name of the list we're iterating on
> + * @__local_list: A pointer to the local list used to store already iterated
> items
> + * @__prev_vm_bo: The previous element we got from
> drm_gpuvm_get_next_cached_vm_bo()
> + *
> + * This helper is here to provide lockless list iteration. Lockless as in,
> the
> + * iterator releases the lock immediately after picking the first element
> from
> + * the list, so list insertion deletion can happen concurrently.
> + *
> + * Elements popped from the original list are kept in a local list, so
> removal
> + * and is_empty checks can still happen while we're iterating the list.
> + */
> +#define get_next_vm_bo_from_list(__gpuvm, __list_name, __local_list,
> __prev_vm_bo) \
> + ({
> \
> + struct drm_gpuvm_bo *__vm_bo;
> \
Missing NULL assignment here.
> +
> \
> + drm_gpuvm_bo_put(__prev_vm_bo);
> \
> +
> \
> + spin_lock(&(__gpuvm)->__list_name.lock);
> \
> + while (!list_empty(&(__gpuvm)->__list_name.list)) {
> \
> + __vm_bo =
> list_first_entry(&(__gpuvm)->__list_name.list, \
> + struct drm_gpuvm_bo,
> \
> + list.entry.__list_name);
> \
> + if (drm_gpuvm_bo_get_unless_zero(__vm_bo)) {
> \
> +
> list_move_tail(&(__vm_bo)->list.entry.__list_name, \
> + __local_list);
> \
> + break;
> \
> + } else {
> \
> +
> list_del_init(&(__vm_bo)->list.entry.__list_name); \
> + __vm_bo = NULL;
> \
> + }
> \
> + }
> \
> + spin_unlock(&(__gpuvm)->__list_name.lock);
> \
> +
> \
> + __vm_bo;
> \
> + })
> +
> +/**
> + * for_each_vm_bo_in_list() - internal vm_bo list iterator
> + *
> + * This helper is here to provide lockless list iteration. Lockless as in,
> the
> + * iterator releases the lock immediately after picking the first element
> from the
> + * list, so list insertion and deletion can happen concurrently.
> + *
> + * Typical use:
> + *
> + * struct drm_gpuvm_bo *vm_bo;
> + * LIST_HEAD(my_local_list);
> + *
> + * ret = 0;
> + * drm_gpuvm_for_each_vm_bo(gpuvm, <list_name>, &my_local_list, vm_bo) {
> + * ret = do_something_with_vm_bo(..., vm_bo);
> + * if (ret)
> + * break;
> + * }
> + * drm_gpuvm_bo_put(vm_bo);
> + * drm_gpuvm_restore_vm_bo_list(gpuvm, <list_name>, &my_local_list);
Might be worth mentioning that the vm_bo pointer shouldn't be
re-assigned from inside for loop, otherwise
the next get_next_vm_bo_from_list() will be passed a wrong prev_vm_bo.
> + *
> + *
> + * Only used for internal list iterations, not meant to be exposed to the
> outside
> + * world.
> + */
> +#define for_each_vm_bo_in_list(__gpuvm, __list_name, __local_list, __vm_bo)
> \
> + for (__vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name,
> \
> + __local_list, NULL);
> \
> + __vm_bo;
> \
> + __vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name,
> \
> + __local_list, __vm_bo))
> \
