Re: [PATCH v6] drm/ttm/pool: back up at native page order

[sashiko-bot@kernel.org]

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 <matthew.brost@intel.com>

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/20260617021851.1164946-1-matthew.brost@intel.com?part=1