Re: [PATCH 03/10] drm/ttm: remove ttm_lru_walk_ops

[Matthew Brost <matthew.brost@intel.com>]

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 <christian.koenig@amd.com>
> >>
> >> It's just another layer of indirection.
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
> > 
> > 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;
> >>  };
>