Re: [PATCH v3 5/8] mm: zswap: Store the folio's node id in the zswap_entry.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm: zswap: shrink_memcg() skips entries on nodes outside
  N_NORMAL_MEMORY
--

commit f6e1c6117279d2be409376eb57c05492fc654dea
Author: Kanchana P. Sridhar <kanchanapsridhar2026@gmail.com>

mm: zswap: Store the folio's node id in the zswap_entry.

This commit stores the swapped-out folio's NUMA node ID in struct
zswap_entry for list_lru tracking and switches zswap_entry slab allocations
to be node-agnostic.

> diff --git a/mm/zswap.c b/mm/zswap.c
> index 78cf3d16b97b0..95997172db6f6 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
[ ... ]
> @@ -680,14 +685,8 @@ static int zswap_enabled_param_set(const char *val,
>  * lru functions
>  **********************************/
>  
> -static inline int entry_to_nid(struct zswap_entry *entry)
> -{
> -	return page_to_nid(virt_to_page(entry));
> -}
> -
>  static void zswap_lru_add(struct zswap_entry *entry)
>  {
> -	int nid = entry_to_nid(entry);
>  	struct mem_cgroup *memcg;
>  
>  	/*
> @@ -704,19 +703,18 @@ static void zswap_lru_add(struct zswap_entry *entry)
>  	rcu_read_lock();
>  	memcg = obj_cgroup_memcg(entry->objcg);
>  	/* will always succeed */
> -	list_lru_add(&zswap_list_lru, &entry->lru, nid, memcg);
> +	list_lru_add(&zswap_list_lru, &entry->lru, entry->nid, memcg);
>  	rcu_read_unlock();
>  }
[ ... ]
> @@ -1539,9 +1538,10 @@ static bool zswap_store_page(struct folio *folio, long index,
>  {
>  	swp_entry_t page_swpentry = folio_swap_entry(folio, index);
>  	struct zswap_entry *entry, *old;
> +	int nid = folio_nid(folio);
>  
>  	/* allocate entry */
> -	entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio));
> +	entry = zswap_entry_cache_alloc(GFP_KERNEL);
>  	if (!entry) {
>  		zswap_reject_kmemcache_fail++;
>  		return false;
> @@ -1556,6 +1556,7 @@ static bool zswap_store_page(struct folio *folio, long index,
>  	 * to an unrelated live pool.
>  	 */
>  	entry->pool_idx = pool->idx;
> +	entry->nid = nid;

[Severity: Medium]
Can shrink_memcg() miss zswap_list_lru entries when a folio comes from a
node outside N_NORMAL_MEMORY?

Previously, entry_to_nid() derived the list_lru node ID from the GFP_KERNEL
slab page backing struct zswap_entry, which was guaranteed to be on a node
in N_NORMAL_MEMORY.  Now entry->nid is set to folio_nid(folio), and movable
swap-backed folios can reside on ZONE_MOVABLE-only or ZONE_HIGHMEM-only
nodes that are in N_MEMORY but not in N_NORMAL_MEMORY.  Because
shrink_memcg() only walks N_NORMAL_MEMORY nodes:

mm/zswap.c:shrink_memcg() {
    ...
	for_each_node_state(nid, N_NORMAL_MEMORY) {
		unsigned long nr_to_walk = SWAP_CLUSTER_MAX;

		shrunk += list_lru_walk_one(&zswap_list_lru, nid, memcg,
					    &shrink_memcg_cb, &ctl, &nr_to_walk);
		scanned += SWAP_CLUSTER_MAX - nr_to_walk;
	}
    ...
}

entries added under entry->nid for nodes outside N_NORMAL_MEMORY are
skipped by both per-cgroup and global pool limit reclaim, leaving them
unreclaimable by shrink_memcg() and causing premature zswap_store()
rejections once limits are reached.  Should shrink_memcg() iterate over
N_MEMORY instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106072412.7909-1-kanchanapsridhar2026@gmail.com?part=5