Re: [PATCH 3/3] mm: page_counter: drop protection fields from struct page_counter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Orphaned and factually incorrect comment and redundant padding in `struct page_counter`.
--

commit 2561d898691e5e4e0a87e8f8b40818bf2f82cbb1
Author: linuszeng <linuszeng@tencent.com>

mm: page_counter: drop protection fields from struct page_counter

This commit drops protection-related fields from struct page_counter and
moves them into a new struct page_counter_protection. This shrinks the
page_counter structure for non-memory counters like swap and kmem.

> diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
> index b81f16702764e..a4dd461523af2 100644
> --- a/include/linux/page_counter.h
> +++ b/include/linux/page_counter.h
[ ... ]
> @@ -73,6 +57,9 @@ struct page_counter {
>  	 * support memory.min/memory.low (swap, memsw, kmem, tcpmem, ...).
>  	 */
>  	struct page_counter_protection *prot;
> +
> +	/* Keep all the read most fields in a separete cacheline. */
> +	CACHELINE_PADDING(_pad2_);
>  } ____cacheline_internodealigned_in_smp;

[Severity: Low]
Does this move of the padding to the end of the struct render both the
comment incorrect and the padding redundant?

Because the struct is already decorated with
____cacheline_internodealigned_in_smp, adding CACHELINE_PADDING(_pad2_) at the
very end appears unnecessary.

Additionally, since the padding was moved, the read-mostly fields (such as
track_failcnt, high, max) now share the second cacheline with the frequently
modified watermark fields. This behavior contradicts the comment that
states it keeps all the read most fields in a separate cacheline.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-descriptive-name-v1-0-1828961cb01a@tencent.com?part=3