Received-SPF: pass (google.com: domain of linux-kernel+bounces-170310-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240506170024.202111-1-yosryahmed@google.com> <d546b804-bb71-45c7-89c4-776f98a48e77@redhat.com>
In-Reply-To: <d546b804-bb71-45c7-89c4-776f98a48e77@redhat.com>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Mon, 6 May 2024 11:52:40 -0700
Message-ID: <CAJD7tkaPvYWhrMSHmM0n0hitFaDdusq7gQ=7+DTUQLODGdo6RQ@mail.gmail.com>
Subject: Re: [PATCH] mm: do not update memcg stats for NR_{FILE/SHMEM}_PMDMAPPED
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, Shakeel Butt <shakeel.butt@linux.dev>, 
	Johannes Weiner <hannes@cmpxchg.org>, Michal Hocko <mhocko@kernel.org>, 
	Roman Gushchin <roman.gushchin@linux.dev>, Muchun Song <muchun.song@linux.dev>, linux-mm@kvack.org, 
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, 
	syzbot+9319a4268a640e26b72b@syzkaller.appspotmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, May 6, 2024 at 11:35=E2=80=AFAM David Hildenbrand <david@redhat.com=
> wrote:
>
> On 06.05.24 19:00, Yosry Ahmed wrote:
> > Do not use __lruvec_stat_mod_folio() when updating NR_FILE_PMDMAPPED an=
d
> > NR_SHMEM_PMDMAPPED as these stats are not maintained per-memcg. Use
> > __mod_node_page_state() instead, which updates the global per-node stat=
s
> > only.
>
> What's the effect of this? IIUC, it's been that way forever, no?

Yes, but it has been the case that all the NR_VM_EVENT_ITEMS stats
were maintained per-memcg, although some of those fields are not
exposed anywhere.

Shakeel recently added commit14e0f6c957e39 ("memcg: reduce memory for
the lruvec and memcg stats"), which changed this such that we only
maintain the stats we actually expose per-memcg (via a translation
table).

He also added commit 514462bbe927b ("memcg: warn for unexpected events
and stats"), which warns if we try to update a stat per-memcg that we
do not maintain per-memcg (i.e. the warning firing here). The goal is
to make sure the translation table has all the stats it needs to have.

Both of these commits were just merged today into mm-stable, hence the
need for the fix now. It is the warning working as intended. No Fixes
or CC stable are needed, but if necessary I would think:

Fixes: 514462bbe927b ("memcg: warn for unexpected events and stats")

, because without the warning, the stat update will just be ignored.
So if anything the warning should have been added *after* this was
fixed up.

>
> Fixes: ?
>
> Do we want to CC stable?
>
>
> --
> Cheers,
>
> David / dhildenb
>