Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] Redundant Writeback IO and Reclaim Degradation due to Missing Dirty 
Flag Clear
- [Medium] Inaccurate Watermark Check due to Counter Drift
--

commit c4c05c6bea6e7dbeb49a19a207e3cc6b5e0bc762
Author: Matthew Brost <[email protected]>

drm/ttm/pool: back up at native page order

This commit modifies the TTM pool backup process to handle compound pages
at their native order instead of unconditionally splitting them into order-0
pages. It introduces a new folio backup helper to back up contiguous
subpages to shmem, alongside a proactive splitting mechanism to split
pages when under memory pressure.

> diff --git a/drivers/gpu/drm/ttm/ttm_backup.c 
> b/drivers/gpu/drm/ttm/ttm_backup.c
> index 81df4cb5606b4..e67921393f56c 100644
> --- a/drivers/gpu/drm/ttm/ttm_backup.c
> +++ b/drivers/gpu/drm/ttm/ttm_backup.c
[ ... ]
> @@ -87,53 +93,66 @@ int ttm_backup_copy_page(struct file *backup, struct page 
> *dst,
[ ... ]
> +             folio_mark_accessed(to_folio);
> +             folio_lock(to_folio);
> +             folio_mark_dirty(to_folio);
> +
> +             for (j = 0; j < to_nr; j++)
> +                     copy_highpage(folio_file_page(to_folio, idx + i + j),
> +                                   folio_page(folio, i + j));
> +
> +             if (writeback && !folio_mapped(to_folio)) {

[Severity: High]
Does this code skip clearing the dirty flag for IO before calling
shmem_writeout()?

The previous implementation of ttm_backup_backup_page() called
folio_clear_dirty_for_io() and folio_set_reclaim() before starting
the writeback. Without clearing the dirty flag first,
__folio_start_writeback() will not clear the PAGECACHE_TAG_DIRTY mark.

As a result, even if the content is successfully written, the folio
remains marked as dirty, potentially leading to kswapd repeatedly
attempting to write it out again.

> +                     ret = shmem_writeout(to_folio, NULL, NULL);
> +                     if (ret == AOP_WRITEPAGE_ACTIVATE)
> +                             folio_unlock(to_folio);
> +             } else {
>                       folio_unlock(to_folio);
>       }
>  
> -     folio_put(to_folio);
> +             folio_put(to_folio);
> +             i += to_nr;
> +             *nr_pages_backed = i;
> +     }
>  
> -     return handle;
> +     return ttm_backup_shmem_idx_to_handle(idx);
>  }
>  
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 278bbe7a11add..d76bfe527ac79 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
[ ... ]
> @@ -1010,6 +1010,38 @@ void ttm_pool_drop_backed_up(struct ttm_tt *tt)
>       ttm_pool_free_range(NULL, tt, ttm_cached, start_page, tt->num_pages);
>  }
>  
> +static bool ttm_pool_split_for_nearly_oom(struct ttm_pool *pool,
> +                                       struct page *page)
> +{
> +     unsigned int order = ttm_pool_page_order(pool, page);
> +     int nid = pool->nid;
> +     enum zone_type zone_type;
> +
> +     if (!order)
> +             return false;
> +
> +     if (!numa_valid_node(nid))
> +             return false;
> +
> +#if IS_ENABLED(CONFIG_ZONE_DMA32)
> +     zone_type = ZONE_DMA32;
> +#else
> +     zone_type = ZONE_NORMAL;
> +#endif
> +
> +     for (; zone_type <= ZONE_NORMAL; ++zone_type) {
> +             struct zone *zone = &NODE_DATA(nid)->node_zones[zone_type];
> +
> +             if (zone_page_state(zone, NR_FREE_PAGES) <

[Severity: Medium]
Can directly reading zone_page_state() miss per-CPU counter drift?

The zone_page_state() function omits per-CPU deltas, and on systems
with many CPUs, this error can exceed the actual watermark gaps.

Using it directly rather than zone_page_state_snapshot() or higher-level
watermark APIs might cause this heuristic to read artificially high
free page counts, delaying the proactive split optimization under
sustained memory pressure.

> +                 low_wmark_pages(zone) / 2) {
> +                     ttm_pool_split_for_swap(pool, page);
> +                     return true;
> +             }
> +     }
> +
> +     return false;
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to