Re: [PATCH v3 2/4] mm: add vmstat for cgroup uncharged pages

[Boris Burkov <boris@bur.io>]

On Tue, Aug 19, 2025 at 03:50:49AM +0100, Matthew Wilcox wrote:
> On Mon, Aug 18, 2025 at 05:36:54PM -0700, Boris Burkov wrote:
> > Uncharged pages are tricky to track by their essential "uncharged"
> > nature. To maintain good accounting, introduce a vmstat counter tracking
> > all uncharged pages. Since this is only meaningful when cgroups are
> > configured, only expose the counter when CONFIG_MEMCG is set.
> 
> I don't understand why this is needed.  Maybe Shakeel had better
> reasoning that wasn't captured in the commit message.
> 
> If they're unaccounted, then you can get a good estimate of them
> just by subtracting the number of accounted pages from the number of
> file pages.  Sure there's a small race between the two numbers being
> updated, so you migth be off by a bit.

I don't think there is any over the top elaborate reasoning beyond being
precise and accurate with stats being a good thing.

In my experience, implicit calculations like the one you propose tend to
lead to metrics that drift into incorrectness. Today's correct
calculation is tomorrow's wrong one, when the invariants from which it
was derived shift again. We have seen this in practice before with
kernel memory usage at Meta.

I don't think this costs a lot, and it has an easy to understand
definition. Are you concerned that there is only a single user in btrfs,
so that doesn't merit defining a new stat?

Sorry to put words in your mouth, just trying to guess what might be
objectionable about it.

Thanks for the reviews, by the way.