[PATCH v3 02/21] drm/ttm: Slightly clean up LRU list iteration

[Matthew Brost]

On Tue, May 21, 2024 at 09:29:35AM -0600, Saarinen, Jani wrote:
> Hi, 
> > -----Original Message-----
> > From: Intel-gfx-trybot <intel-gfx-trybot-bounces at lists.freedesktop.org> On
> > Behalf Of Matthew Brost
> > Sent: Tuesday, 21 May 2024 17.35
> > To: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > Cc: intel-gfx-trybot at lists.freedesktop.org
> > Subject: Re: [PATCH v3 02/21] drm/ttm: Slightly clean up LRU list iteration
> > 
> > On Tue, May 21, 2024 at 09:53:05AM +0200, Thomas Hellström wrote:
> > > To make the transition to using lru hitches easier, simplify the
> > > ttm_resource_manager_next() interface to only take the cursor and
> > > reuse ttm_resource_manager_next() functionality from
> > > ttm_resource_manager_first().
> > >
> > > Cc: Christian König <christian.koenig at amd.com>
> > > Cc: Somalapuram Amaranath <Amaranath.Somalapuram at amd.com>
> > > Cc: Matthew Brost <matthew.brost at intel.com>
> > 
> > Reviewed-by: Matthew Brost <matthew.brost at intel.com>
> I assume we should not review on trybot or? 

This was not my intention, replied to wrong version...

Matt

> > 
> > > Cc: <dri-devel at lists.freedesktop.org>
> > > Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/ttm/ttm_resource.c | 48 +++++++++++++-----------------
> > >  include/drm/ttm/ttm_resource.h     | 10 ++++---
> > >  2 files changed, 27 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > > b/drivers/gpu/drm/ttm/ttm_resource.c
> > > index db9a7a3717c4..8bfbddddc0e8 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > @@ -496,50 +496,44 @@ struct ttm_resource *
> > > ttm_resource_manager_first(struct ttm_resource_manager *man,
> > >  			   struct ttm_resource_cursor *cursor)  {
> > > -	struct ttm_lru_item *lru;
> > > -
> > >  	lockdep_assert_held(&man->bdev->lru_lock);
> > >
> > > -	for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
> > > -	     ++cursor->priority)
> > > -		list_for_each_entry(lru, &man->lru[cursor->priority], link) {
> > > -			if (ttm_lru_item_is_res(lru))
> > > -				return ttm_lru_item_to_res(lru);
> > > -		}
> > > -
> > > -	return NULL;
> > > +	cursor->priority = 0;
> > > +	cursor->man = man;
> > > +	cursor->cur = &man->lru[cursor->priority];
> > > +	return ttm_resource_manager_next(cursor);
> > >  }
> > >
> > >  /**
> > >   * ttm_resource_manager_next
> > >   *
> > > - * @man: resource manager to iterate over
> > >   * @cursor: cursor to record the position
> > > - * @res: the current resource pointer
> > >   *
> > > - * Returns the next resource from the resource manager.
> > > + * Return: the next resource from the resource manager.
> > >   */
> > >  struct ttm_resource *
> > > -ttm_resource_manager_next(struct ttm_resource_manager *man,
> > > -			  struct ttm_resource_cursor *cursor,
> > > -			  struct ttm_resource *res)
> > > +ttm_resource_manager_next(struct ttm_resource_cursor *cursor)
> > >  {
> > > -	struct ttm_lru_item *lru = &res->lru;
> > > +	struct ttm_resource_manager *man = cursor->man;
> > > +	struct ttm_lru_item *lru;
> > >
> > >  	lockdep_assert_held(&man->bdev->lru_lock);
> > >
> > > -	list_for_each_entry_continue(lru, &man->lru[cursor->priority], link) {
> > > -		if (ttm_lru_item_is_res(lru))
> > > -			return ttm_lru_item_to_res(lru);
> > > -	}
> > > -
> > > -	for (++cursor->priority; cursor->priority < TTM_MAX_BO_PRIORITY;
> > > -	     ++cursor->priority)
> > > -		list_for_each_entry(lru, &man->lru[cursor->priority], link) {
> > > -			if (ttm_lru_item_is_res(lru))
> > > -				ttm_lru_item_to_res(lru);
> > > +	for (;;) {
> > > +		lru = list_entry(cursor->cur, typeof(*lru), link);
> > > +		list_for_each_entry_continue(lru, &man->lru[cursor-
> > >priority], link) {
> > > +			if (ttm_lru_item_is_res(lru)) {
> > > +				cursor->cur = &lru->link;
> > > +				return ttm_lru_item_to_res(lru);
> > > +			}
> > >  		}
> > >
> > > +		if (++cursor->priority >= TTM_MAX_BO_PRIORITY)
> > > +			break;
> > > +
> > > +		cursor->cur = &man->lru[cursor->priority];
> > > +	}
> > > +
> > >  	return NULL;
> > >  }
> > >
> > > diff --git a/include/drm/ttm/ttm_resource.h
> > > b/include/drm/ttm/ttm_resource.h index 1511d91e290d..7d81fd5b5b83
> > > 100644
> > > --- a/include/drm/ttm/ttm_resource.h
> > > +++ b/include/drm/ttm/ttm_resource.h
> > > @@ -272,11 +272,15 @@ ttm_lru_item_to_res(struct ttm_lru_item *item)
> > >  /**
> > >   * struct ttm_resource_cursor
> > >   *
> > > + * @man: The resource manager currently being iterated over.
> > > + * @cur: The list head the cursor currently points to.
> > >   * @priority: the current priority
> > >   *
> > >   * Cursor to iterate over the resources in a manager.
> > >   */
> > >  struct ttm_resource_cursor {
> > > +	struct ttm_resource_manager *man;
> > > +	struct list_head *cur;
> > >  	unsigned int priority;
> > >  };
> > >
> > > @@ -438,9 +442,7 @@ struct ttm_resource *
> > > ttm_resource_manager_first(struct ttm_resource_manager *man,
> > >  			   struct ttm_resource_cursor *cursor);  struct
> > ttm_resource *
> > > -ttm_resource_manager_next(struct ttm_resource_manager *man,
> > > -			  struct ttm_resource_cursor *cursor,
> > > -			  struct ttm_resource *res);
> > > +ttm_resource_manager_next(struct ttm_resource_cursor *cursor);
> > >
> > >  struct ttm_resource *
> > >  ttm_lru_first_res_or_null(struct list_head *head); @@ -455,7 +457,7
> > > @@ ttm_lru_first_res_or_null(struct list_head *head);
> > >   */
> > >  #define ttm_resource_manager_for_each_res(man, cursor, res)
> > 	\
> > >  	for (res = ttm_resource_manager_first(man, cursor); res;	\
> > > -	     res = ttm_resource_manager_next(man, cursor, res))
> > > +	     res = ttm_resource_manager_next(cursor))
> > >
> > >  struct ttm_kmap_iter *
> > >  ttm_kmap_iter_iomap_init(struct ttm_kmap_iter_iomap *iter_io,
> > > --
> > > 2.44.0
> > >