On Sun, Dec 06, 2020 at 06:14:40PM +0800, Muchun Song wrote:
> The unit of NR_ANON_THPS is HPAGE_PMD_NR already. So it should inc/dec
> by one rather than nr_pages.
This is a real bug, thanks for catching it.
However, your patch changes the user-visible output of /proc/vmstat!
NR_ANON_THPS isn't just used by memcg, it's a generic accounting item
of the memory subsystem. See this from the Fixes:-patch:
- __inc_node_page_state(page, NR_ANON_THPS);
+ __inc_lruvec_page_state(page, NR_ANON_THPS);
While we've considered /proc/vmstat less official than other files
like meminfo, and have in the past freely added and removed items,
changing the unit of an existing one is not going to work.
On Thu, Dec 10, 2020 at 05:00:47PM +0100, Johannes Weiner wrote:
> On Sun, Dec 06, 2020 at 06:14:40PM +0800, Muchun Song wrote:
> > The unit of NR_ANON_THPS is HPAGE_PMD_NR already. So it should inc/dec
> > by one rather than nr_pages.
>
> This is a real bug, thanks for catching it.
>
> However, your patch changes the user-visible output of /proc/vmstat!
>
> NR_ANON_THPS isn't just used by memcg, it's a generic accounting item
> of the memory subsystem. See this from the Fixes:-patch:
>
> - __inc_node_page_state(page, NR_ANON_THPS);
> + __inc_lruvec_page_state(page, NR_ANON_THPS);
>
> While we've considered /proc/vmstat less official than other files
> like meminfo, and have in the past freely added and removed items,
> changing the unit of an existing one is not going to work.
Argh, I hit send instead of cancel after noticing that I misread your
patch completely. Scratch what I wrote above.