Re: [PATCH] mm: don't be stuck to rmap lock on reclaim path

[Matthew Wilcox <willy@infradead.org>]

On Wed, May 04, 2022 at 04:47:51PM -0700, Minchan Kim wrote:
> Since this patch introduces a new "contended" field as out param
> in rmap_walk_control, it's not immutable any longer so remove
> const keywords on rmap related functions. Since rmap walking
> is already expensive operation, I doubt the const would help sizable
> benefit(And we didn't have it until 5.17).

Um?  If it's now mutable, it surely can't be static as that means it
would be shared by all callers, and you might write to the new fields
in one caller and have them interpreted by the other caller!

Or if it is safe, then the comment needs to not be deleted, but modified
to explain why it's safe to do so in this instance, and that other
instances should not copy the approach unless they are similarly safe.

> diff --git a/mm/page_idle.c b/mm/page_idle.c
> index fc0435abf909..fdff8c6dcd2d 100644
> --- a/mm/page_idle.c
> +++ b/mm/page_idle.c
> @@ -86,11 +86,8 @@ static bool page_idle_clear_pte_refs_one(struct folio *folio,
>  static void page_idle_clear_pte_refs(struct page *page)
>  {
>  	struct folio *folio = page_folio(page);
> -	/*
> -	 * Since rwc.arg is unused, rwc is effectively immutable, so we
> -	 * can make it static const to save some cycles and stack.
> -	 */
> -	static const struct rmap_walk_control rwc = {
> +
> +	static struct rmap_walk_control rwc = {
>  		.rmap_one = page_idle_clear_pte_refs_one,
>  		.anon_lock = folio_lock_anon_vma_read,
>  	};