Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing cleanup of `ttm_pool_mgr_init()` allocations when `alloc_page()` fails in `ttm_global_init()`. -- --- Patch [6]: [PATCH v5 6/6] ttm_pool_mgr_init() does not check return value --- commit 18452a40ead8211d272e9f32ad64ceec8ef1cf22 Author: Óscar Megía López <[email protected]> ttm_pool_mgr_init() does not check return value This commit fixes a missing return value check for ttm_pool_mgr_init() in ttm_global_init(). It propagates the error by jumping to the out label if initialization fails. Cc: <[email protected]> # 7.1.0 Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)") > diff --git a/drivers/gpu/drm/ttm/ttm_device.c > b/drivers/gpu/drm/ttm/ttm_device.c > index d3bfb9a696a74..896b766712d04 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -96,7 +96,9 @@ static int ttm_global_init(void) > >> PAGE_SHIFT; > num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT)); > > - ttm_pool_mgr_init(num_pages); > + ret = ttm_pool_mgr_init(num_pages); > + if (ret) > + goto out; [Severity: High] This is a pre-existing issue, but does the error handling at the out label properly clean up after a successful ttm_pool_mgr_init() call? If the dummy_read_page allocation fails later in this function, we jump to the out label returning -ENOMEM: glob->dummy_read_page = alloc_page(__GFP_ZERO); if (unlikely(glob->dummy_read_page == NULL)) { ret = -ENOMEM; goto out; } However, the cleanup at the out label doesn't appear to call ttm_pool_mgr_fini() or unregister the shrinker: out: if (ret && ttm_debugfs_root) debugfs_remove(ttm_debugfs_root); if (ret) --ttm_glob_use_count; mutex_unlock(&ttm_global_mutex); return ret; Since ttm_pool_mgr_init() registers a shrinker (mm_shrinker) and initializes global spinlocks and lists: spin_lock_init(&shrinker_lock); INIT_LIST_HEAD(&shrinker_list); ... mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool"); Could this leave a registered shrinker behind on failure? If ttm_global_init() is called again on a subsequent driver load, it would unconditionally re-initialize shrinker_lock and shrinker_list. If the originally leaked shrinker is running concurrently during memory reclaim, could this cause lock or list corruption? > ttm_tt_mgr_init(num_pages, num_dma32); > > glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32 | -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
