On 7/6/26 14:34, Thomas Hellström wrote:
> Hi,
>
> On Fri, 2026-07-03 at 18:31 +0200, Natalie Vock wrote:
>> From: Christian König <[email protected]>
>>
>> It's just another layer of indirection.
>>
>> Signed-off-by: Christian König <[email protected]>
>> Signed-off-by: Natalie Vock <[email protected]>
>
> Personally I don't have a strong opinion on this, but the reason for
> separating out the ops is that adding function pointers in the walk
> iterator itself was once pushed back on quite forcefully by Linus when
> I tried to do that in mm/pagewalk. Claiming for various reasons the
> standard way of doing that in Linux is using a const ops struct that
> ends up in unmodifiable memory.
Ah! I was already wondering why the extra indirection was used.
I'm perfectly fine to keep it. It just looked a bit odd.
Regards,
Christian.
>
> /Thomas
>
>
>> ---
>> drivers/gpu/drm/ttm/ttm_bo.c | 12 ++----------
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +-
>> include/drm/ttm/ttm_bo.h | 34 ++++++++++++++---------------
>> -----
>> 3 files changed, 17 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 2b470c1746f60..1fb8c53da0362 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -545,10 +545,6 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk
>> *walk, struct ttm_buffer_object *
>> return lret;
>> }
>>
>> -static const struct ttm_lru_walk_ops ttm_evict_walk_ops = {
>> - .process_bo = ttm_bo_evict_cb,
>> -};
>> -
>> static int ttm_bo_evict_alloc(struct ttm_device *bdev,
>> struct ttm_resource_manager *man,
>> const struct ttm_place *place,
>> @@ -560,7 +556,7 @@ static int ttm_bo_evict_alloc(struct ttm_device
>> *bdev,
>> {
>> struct ttm_bo_evict_walk evict_walk = {
>> .walk = {
>> - .ops = &ttm_evict_walk_ops,
>> + .process_bo = ttm_bo_evict_cb,
>> .arg = {
>> .ctx = ctx,
>> .ticket = ticket,
>> @@ -1187,10 +1183,6 @@ ttm_bo_swapout_cb(struct ttm_lru_walk *walk,
>> struct ttm_buffer_object *bo)
>> return ret;
>> }
>>
>> -const struct ttm_lru_walk_ops ttm_swap_ops = {
>> - .process_bo = ttm_bo_swapout_cb,
>> -};
>> -
>> /**
>> * ttm_bo_swapout() - Swap out buffer objects on the LRU list to
>> shmem.
>> * @bdev: The ttm device.
>> @@ -1209,7 +1201,7 @@ s64 ttm_bo_swapout(struct ttm_device *bdev,
>> struct ttm_operation_ctx *ctx,
>> {
>> struct ttm_bo_swapout_walk swapout_walk = {
>> .walk = {
>> - .ops = &ttm_swap_ops,
>> + .process_bo = ttm_bo_swapout_cb,
>> .arg = {
>> .ctx = ctx,
>> .trylock_only = true,
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 7ed085adf1c9b..29f068944a972 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -919,7 +919,7 @@ s64 ttm_lru_walk_for_evict(struct ttm_lru_walk
>> *walk, struct ttm_device *bdev,
>> s64 lret;
>>
>> ttm_bo_lru_for_each_reserved_guarded(&cursor, man, &walk-
>>> arg, bo) {
>> - lret = walk->ops->process_bo(walk, bo);
>> + lret = walk->process_bo(walk, bo);
>> if (lret == -EBUSY || lret == -EALREADY)
>> lret = 0;
>> progress = (lret < 0) ? lret : progress + lret;
>> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
>> index 1eae9eea5ff32..0fcd5082a7080 100644
>> --- a/include/drm/ttm/ttm_bo.h
>> +++ b/include/drm/ttm/ttm_bo.h
>> @@ -189,24 +189,6 @@ struct ttm_operation_ctx {
>> uint64_t bytes_moved;
>> };
>>
>> -struct ttm_lru_walk;
>> -
>> -/** struct ttm_lru_walk_ops - Operations for a LRU walk. */
>> -struct ttm_lru_walk_ops {
>> - /**
>> - * process_bo - Process this bo.
>> - * @walk: struct ttm_lru_walk describing the walk.
>> - * @bo: A locked and referenced buffer object.
>> - *
>> - * Return: Negative error code on error, User-defined
>> positive value
>> - * (typically, but not always, size of the processed bo) on
>> success.
>> - * On success, the returned values are summed by the walk
>> and the
>> - * walk exits when its target is met.
>> - * 0 also indicates success, -EBUSY means this bo was
>> skipped.
>> - */
>> - s64 (*process_bo)(struct ttm_lru_walk *walk, struct
>> ttm_buffer_object *bo);
>> -};
>> -
>> /**
>> * struct ttm_lru_walk_arg - Common part for the variants of BO LRU
>> walk.
>> */
>> @@ -223,8 +205,20 @@ struct ttm_lru_walk_arg {
>> * struct ttm_lru_walk - Structure describing a LRU walk.
>> */
>> struct ttm_lru_walk {
>> - /** @ops: Pointer to the ops structure. */
>> - const struct ttm_lru_walk_ops *ops;
>> + /**
>> + * process_bo - Process this bo.
>> + * @walk: struct ttm_lru_walk describing the walk.
>> + * @bo: A locked and referenced buffer object.
>> + *
>> + * Return: Negative error code on error, User-defined
>> positive value
>> + * (typically, but not always, size of the processed bo) on
>> success.
>> + * On success, the returned values are summed by the walk
>> and the
>> + * walk exits when its target is met.
>> + * 0 also indicates success, -EBUSY means this bo was
>> skipped.
>> + */
>> + s64 (*process_bo)(struct ttm_lru_walk *walk,
>> + struct ttm_buffer_object *bo);
>> +
>> /** @arg: Common bo LRU walk arguments. */
>> struct ttm_lru_walk_arg arg;
>> };
