Re: [RFC PATCH v2 01/13] mm: add CONFIG_ANON_VMA_FRACTAL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Pre-order tree traversal iterators are broken due to heterogeneous depth increments, causing cross-process memory corruption and missed VMAs during reverse mapping.
- [Medium] PAGE_SIZE-dependent VMA sharing limit causes severe memory bloat and arbitrary lock contention scaling.
- [Low] Duplicate and conflicting type directives for ANON_VMA_FRACTAL.
--

--- Patch [1]: [RFC PATCH v2 01/13] mm: add CONFIG_ANON_VMA_FRACTAL ---
commit 7f3f3b3fc54ea6c9180e3a0f624b7af6752d7ca5
Author: tao <tao.wangtao@honor.com>

mm: add CONFIG_ANON_VMA_FRACTAL

This commit adds the CONFIG_ANON_VMA_FRACTAL option to replace the
anon_vma_chain. It uses vma_rmap_base to compute page addresses and
organizes anon_vmas into a fractal list based on fork depth.

> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index 8dc0871e5f001..75738f4ede1f6 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
[ ... ]
> @@ -67,6 +67,54 @@ struct anon_vma {
[ ... ]
> +/* Create rbc child anon_node on VMA remap or when count exceeds MAX. */
> +#define ANON_RMAP_BASE_COUNT_MASK	(PAGE_SIZE - 1)
> +#define ANON_RMAP_BASE_COUNT_MAX	(ANON_RMAP_BASE_COUNT_MASK)
> +
> +/*
> + * Allow VMAs to share an anon_node only if the number of VMAs attached
> + * is below this limit; keep it small to reduce lock contention.
> + */
> +#define ANON_NODE_SHARE_LIMIT	(ANON_RMAP_BASE_COUNT_MAX / 1000)

[Severity: Medium]
Since ANON_RMAP_BASE_COUNT_MAX relies on PAGE_SIZE, does this create an
artificially low sharing limit on 4K systems?

On a system with 4K pages, ANON_RMAP_BASE_COUNT_MAX would evaluate to 4095.
Dividing by 1000 means ANON_NODE_SHARE_LIMIT becomes 4, which would force a
new anon_node allocation very frequently and could cause memory bloat.

> +
> +#endif
> +
>  /*
>   * The copy-on-write semantics of fork mean that an anon_vma
>   * can become associated with multiple processes. Furthermore,
> diff --git a/mm/Kconfig b/mm/Kconfig
> index f794a341d7422..e0a1e96c6835a 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1451,6 +1451,34 @@ config LOCK_MM_AND_FIND_VMA
>  	bool
>  	depends on !STACK_GROWSUP
>  
> +config ANON_VMA_FRACTAL
> +	bool "anon_vma fractal list"
> +	def_bool n

[Severity: Low]
This isn't a bug, but using both a bool prompt and def_bool together
conflicts with standard Kconfig syntax. Could we just use bool and
default n instead?

> +	depends on MMU
> +	help
> +	  This option uses vma_rmap_base(vma) during rmap to compute the page
[ ... ]
> diff --git a/mm/internal.h b/mm/internal.h
> index 841c27611627e..694d3194a6baf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
[ ... ]
> @@ -252,6 +262,24 @@ static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
>  	up_read(&anon_vma->root->rwsem);
>  }
>  
> +#ifdef CONFIG_ANON_VMA_FRACTAL
> +
> +static inline struct anon_node *anon_node_next_rbc_child(
> +		struct anon_node *anon_nod, struct anon_node *node)
> +{
> +	node = list_next_entry(node, fractal_list);
> +	return node->depth == anon_nod->depth + 1 ? node : NULL;
> +}

[Severity: Critical]
Will this iteration abort prematurely if the next entry in the list is a
descendant of the RBC child?

If an RBC child has its own descendants, they would immediately follow it in
the list. Since anon_node_next_rbc_child() in mm/internal.h strictly expects
the very next list entry to have depth == anon_nod->depth + 1, it appears it
would return NULL and stop iteration early, potentially missing VMAs.

> +
> +static inline struct anon_node *anon_node_next_descendant(
> +		struct anon_node *anon_nod, struct anon_node *node)
> +{
> +	node = list_next_entry(node, fractal_list);
> +	return node->depth > anon_nod->depth ? node : NULL;
> +}

[Severity: Critical]
Could this traversal yield unrelated siblings as descendants when depth
increments are mixed?

Since RBC children increment depth by 1 and fork children increment by 2, a
process calling mremap (creating an RBC child) followed by fork (creating a
fork child) will create mixed increments.

If we iterate descendants of an RBC child (depth P+1),
anon_node_next_descendant() in mm/internal.h would continue as long as
node->depth > anon_nod->depth. A sibling fork child with depth P+2 would
satisfy this condition and incorrectly be yielded as a descendant,
potentially leading to cross-process memory corruption during rmap
traversals.

> +
> +#endif
> +
>  struct anon_vma *folio_get_anon_vma(const struct folio *folio);
>  
>  /* Operations which modify VMAs. */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707063308.29176-1-tao.wangtao@honor.com?part=1