Re: [PATCH 2/4] drm/ttm: Use LRU hitches

2024-02-16 Thread kernel test robot
Hi Thomas,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next 
drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.8-rc4 
next-20240216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Hellstr-m/drm-ttm-Allow-TTM-LRU-list-nodes-of-different-types/20240216-211801
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20240216131446.101961-3-thomas.hellstrom%40linux.intel.com
patch subject: [PATCH 2/4] drm/ttm: Use LRU hitches
config: m68k-allmodconfig 
(https://download.01.org/0day-ci/archive/20240217/202402171115.elbabdca-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240217/202402171115.elbabdca-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202402171115.elbabdca-...@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/ttm/ttm_resource.c:58: warning: expecting prototype for 
>> ttm_resource_cursor_fini_locked(). Prototype was for 
>> ttm_resource_cursor_fini() instead


vim +58 drivers/gpu/drm/ttm/ttm_resource.c

48  
49  /**
50   * ttm_resource_cursor_fini_locked() - Finalize the LRU list cursor 
usage
51   * @cursor: The struct ttm_resource_cursor to finalize.
52   *
53   * The function pulls the LRU list cursor off any lists it was previusly
54   * attached to. Needs to be called without the LRU list lock held. The
55   * function can be called multiple times after eachother.
56   */
57  void ttm_resource_cursor_fini(struct ttm_resource_cursor *cursor)
  > 58  {
59  spinlock_t *lru_lock = >man->bdev->lru_lock;
60  
61  spin_lock(lru_lock);
62  ttm_resource_cursor_fini_locked(cursor);
63  spin_unlock(lru_lock);
64  }
65  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


[PATCH 2/4] drm/ttm: Use LRU hitches

2024-02-16 Thread Thomas Hellström
Have iterators insert themselves into the list they are iterating
over using hitch list nodes. Since only the iterator owner
can remove these list nodes from the list, it's safe to unlock
the list and when continuing, use them as a starting point. Due to
the way LRU bumping works in TTM, newly added items will not be
missed, and bumped items will be iterated over a second time before
reaching the end of the list.

The exception is list with bulk move sublists. When bumping a
sublist, a hitch that is part of that sublist will also be moved
and we might miss items if restarting from it. This will be
addressed in a later patch.

Cc: Christian König 
Cc: 
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/ttm/ttm_bo.c   |  1 +
 drivers/gpu/drm/ttm/ttm_device.c   |  9 ++-
 drivers/gpu/drm/ttm/ttm_resource.c | 94 --
 include/drm/ttm/ttm_resource.h | 16 +++--
 4 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 96a724e8f3ff..2a048b940840 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -622,6 +622,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
if (locked)
dma_resv_unlock(res->bo->base.resv);
}
+   ttm_resource_cursor_fini_locked();
 
if (!bo) {
if (busy_bo && !ttm_bo_get_unless_zero(busy_bo))
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index f27406e851e5..e8a6a1dab669 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -169,12 +169,17 @@ int ttm_device_swapout(struct ttm_device *bdev, struct 
ttm_operation_ctx *ctx,
num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
-   if (!ret)
+   if (!ret) {
+   ttm_resource_cursor_fini();
return num_pages;
-   if (ret != -EBUSY)
+   }
+   if (ret != -EBUSY) {
+   ttm_resource_cursor_fini();
return ret;
+   }
}
}
+   ttm_resource_cursor_fini_locked();
spin_unlock(>lru_lock);
return 0;
 }
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
b/drivers/gpu/drm/ttm/ttm_resource.c
index 0b8beb356b8a..911364e0a5fd 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -32,6 +32,37 @@
 
 #include 
 
+/**
+ * ttm_resource_cursor_fini_locked() - Finalize the LRU list cursor usage
+ * @cursor: The struct ttm_resource_cursor to finalize.
+ *
+ * The function pulls the LRU list cursor off any lists it was previusly
+ * attached to. Needs to be called with the LRU lock held. The function
+ * can be called multiple times after eachother.
+ */
+void ttm_resource_cursor_fini_locked(struct ttm_resource_cursor *cursor)
+{
+   lockdep_assert_held(>man->bdev->lru_lock);
+   list_del_init(>hitch.link);
+}
+
+/**
+ * ttm_resource_cursor_fini_locked() - Finalize the LRU list cursor usage
+ * @cursor: The struct ttm_resource_cursor to finalize.
+ *
+ * The function pulls the LRU list cursor off any lists it was previusly
+ * attached to. Needs to be called without the LRU list lock held. The
+ * function can be called multiple times after eachother.
+ */
+void ttm_resource_cursor_fini(struct ttm_resource_cursor *cursor)
+{
+   spinlock_t *lru_lock = >man->bdev->lru_lock;
+
+   spin_lock(lru_lock);
+   ttm_resource_cursor_fini_locked(cursor);
+   spin_unlock(lru_lock);
+}
+
 /**
  * ttm_lru_bulk_move_init - initialize a bulk move structure
  * @bulk: the structure to init
@@ -475,62 +506,63 @@ void ttm_resource_manager_debug(struct 
ttm_resource_manager *man,
 EXPORT_SYMBOL(ttm_resource_manager_debug);
 
 /**
- * ttm_resource_manager_first
- *
- * @man: resource manager to iterate over
+ * ttm_resource_manager_next() - Continue iterating over the resource manager
+ * resources
  * @cursor: cursor to record the position
  *
- * Returns the first resource from the resource manager.
+ * Return: The next resource from the resource manager.
  */
 struct ttm_resource *
-ttm_resource_manager_first(struct ttm_resource_manager *man,
-  struct ttm_resource_cursor *cursor)
+ttm_resource_manager_next(struct ttm_resource_cursor *cursor)
 {
+   struct ttm_resource_manager *man = cursor->man;
struct ttm_lru_item *lru;
 
lockdep_assert_held(>bdev->lru_lock);
 
-   for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
-++cursor->priority)
-   list_for_each_entry(lru, >lru[cursor->priority], link) {
-   if (ttm_lru_item_is_res(lru))
+