Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [High] Shmem backing pages (handles) are leaked during a purge operation. Pre-existing issues: - [High] If `ttm_pool_backup` fails partially, it results in a double unmap of DMA addresses and mismatched unmap sizes, violating DMA API constraints. - [High] Backing up a TT via `ttm_tt_backup` frees its physical pages to the system but fails to decrement the TTM global `ttm_pages_allocated` counter, causing a permanent accounting leak. -- --- Patch [1]: Patch "drm/ttm: Account for NULL and handle pages in ttm_pool_backup" has been added to the 6.18-stable tree --- commit b4738a9fdee6c750a497cb7d46bafb43babc2b86 Author: [email protected] <[email protected]> Subject: Patch "drm/ttm: Account for NULL and handle pages in ttm_pool_backup" has been added to the 6.18-stable tree This commit updates ttm_pool_backup() to account for NULL pages and backup handles when shrinking pages. It adds a dedicated writeback pass that skips these entries and applies the same guard to the DMA unmap and purge loop. Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index baf27c70a4193..01083720e4e7f 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c [ ... ] > @@ -991,7 +1013,7 @@ long ttm_pool_backup(struct ttm_pool *pool, struct > ttm_tt *tt, > unsigned int order; > > page = tt->pages[i]; > - if (unlikely(!page)) { > + if (unlikely(!page || > ttm_backup_page_ptr_is_handle(page))) { > num_pages = 1; > continue; > } [Severity: High] If this operation is a purge and ttm_backup_page_ptr_is_handle(page) is true, does this skip the handle without freeing its underlying backup resources (shmem folios)? When ttm_pool_backup() is called by the shrinker with flags->purge = true on a TT that has been partially or fully swapped to shmem, the new ttm_backup_page_ptr_is_handle(page) check unconditionally skips the handles. It seems it fails to call ttm_backup_drop(backup, handle) to free the underlying shmem folio, leaving the handles stranded in tt->pages until the BO is eventually destroyed. Will this leak shmem folios? > > order = ttm_pool_page_order(pool, page); > num_pages = 1UL << order; > if (tt->dma_address) > ttm_pool_unmap(pool, tt->dma_address[i], > num_pages); [Severity: High] This is a pre-existing issue, but does this partial failure path lead to double DMA unmaps? If ttm_pool_backup() fails partially (e.g., ttm_backup_backup_page() fails on the first page), the function aborts and returns an error. Because shrunken == 0, ttm_tt_backup() does not set TTM_TT_FLAG_BACKED_UP. The loop here unmaps DMA addresses for all valid pages but does not clear tt->dma_address[i] or tt->pages[i] unless flags->purge is true. The pages remain in tt->pages with uncleared DMA addresses. When the TT is subsequently destroyed via ttm_tt_unpopulate(), it calls ttm_pool_free_range(), which again calls ttm_pool_unmap() on the already unmapped DMA addresses: drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_unmap_and_free() { ... if (dma_addr) ttm_pool_unmap(pool, *dma_addr, nr); ... } Additionally, because the first page's order was cleared via ttm_pool_split_for_swap(), will it be unmapped as 1 page instead of its original mapped size, causing a mismatched unmap size? [Severity: High] This is a pre-existing issue, but does backing up a TT via ttm_tt_backup() leak the TTM global ttm_pages_allocated counter? When ttm_tt_backup() successfully backs up pages, it clears TTM_TT_FLAG_PRIV_POPULATED and sets TTM_TT_FLAG_BACKED_UP without decrementing the ttm_pages_allocated counter: drivers/gpu/drm/ttm/ttm_tt.c:ttm_tt_backup() { ... tt->page_flags &= ~TTM_TT_FLAG_PRIV_POPULATED; tt->page_flags |= TTM_TT_FLAG_BACKED_UP; ... } Later, when the TT is destroyed, ttm_tt_unpopulate() checks ttm_tt_is_populated(): drivers/gpu/drm/ttm/ttm_tt.c:ttm_tt_unpopulate() { ... if (!ttm_tt_is_populated(ttm)) return; ... } Because the PRIV_POPULATED flag is clear, this function returns early and fails to execute the atomic_long_sub(tt->num_pages, &ttm_pages_allocated) decrement at the end of the function. The backup cleanup path (ttm_pool_drop_backed_up()) also never decrements this counter. Does this cause a permanent memory accounting leak? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/2026072924-debate-parched-01e5@gregkh?part=1
