On Mon, Jul 06, 2026 at 03:05:54PM +0200, Christian König wrote:
> 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.
+1, I was about to comment on this patch saying iirc I a read Linus
statement once const ops struct was Linux prefered way.
Matt
>
> 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;
> >> };
>
