Re: [PATCH v16 08/11] secretmem: add memcg accounting

From: Roman Gushchin
Date: Wed Jan 27 2021 - 13:44:10 EST


On Tue, Jan 26, 2021 at 04:05:55PM +0100, Michal Hocko wrote:
> On Tue 26-01-21 14:48:38, Matthew Wilcox wrote:
> > On Mon, Jan 25, 2021 at 11:38:17PM +0200, Mike Rapoport wrote:
> > > I cannot use __GFP_ACCOUNT because cma_alloc() does not use gfp.
> > > Besides, kmem accounting with __GFP_ACCOUNT does not seem
> > > to update stats and there was an explicit request for statistics:
> > >
> > > https://lore.kernel.org/lkml/CALo0P13aq3GsONnZrksZNU9RtfhMsZXGWhK1n=xYJWQizCd4Zw@xxxxxxxxxxxxxx/
> > >
> > > As for (ab)using NR_SLAB_UNRECLAIMABLE_B, as it was already discussed here:
> > >
> > > https://lore.kernel.org/lkml/20201129172625.GD557259@xxxxxxxxxx/
> > >
> > > I think that a dedicated stats counter would be too much at the moment and
> > > NR_SLAB_UNRECLAIMABLE_B is the only explicit stat for unreclaimable memory.
> >
> > That's not true -- Mlocked is also unreclaimable. And doesn't this
> > feel more like mlocked memory than unreclaimable slab? It's also
> > Unevictable, so could be counted there instead.
>
> yes, that is indeed true, except the unreclaimable counter is tracking
> the unevictable LRUs. These pages are not on any LRU and that can cause
> some confusion. Maybe they shouldn't be so special and they should live
> on unevistable LRU and get their stats automagically.
>
> I definitely do agree that this would be a better fit than NR_SLAB
> abuse. But considering that this is somehow even more special than mlock
> then a dedicated counter sounds as even better fit.

I think it depends on how large these areas will be in practice.
If they will be measured in single or double digits MBs, a separate entry
is hardly a good choice: because of the batching the displayed value
will be in the noise range, plus every new vmstat item adds to the
struct mem_cgroup size.

If it will be measured in GBs, of course, a separate counter is preferred.
So I'd suggest to go with NR_SLAB (which should have been named NR_KMEM)
as now and conditionally switch to a separate counter later.

Thanks!