Re: [PATCH] mm: correctly charge compressed memory to its memcg

[Shakeel Butt <shakeelb@google.com>]

On Mon, Aug 15, 2022 at 10:56 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Mon, Aug 15, 2022 at 10:47 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Mon, Aug 15, 2022 at 09:16:08AM -0700, Yosry Ahmed wrote:
> > > On Mon, Aug 15, 2022 at 8:42 AM Yosry Ahmed <yosryahmed@google.com> wrote:
> > > >
> > > > On Mon, Aug 15, 2022 at 8:31 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > > > >
> > > > > On Mon, Aug 15, 2022 at 06:46:46AM -0700, Yosry Ahmed wrote:
> > > > > > Yeah I understand this much, what I don't understand is why we charge
> > > > > > the zswap memory through objcg (thus tying it to memcg kmem charging)
> > > > > > rather than directly through memcg.
> > > > >
> > > > > The charged quantities are smaller than a page, so we have to use the
> > > > > byte interface.
> > > > >
> > > > > The byte interface (objcg) was written for slab originally, hence the
> > > > > link to the kmem option. But note that CONFIG_MEMCG_KMEM is no longer
> > > > > a user-visible option, and for all intents and purposes a fixed part
> > > > > of CONFIG_MEMCG.
> > > > >
> > > > > (There is the SLOB quirk. But I'm not sure anybody uses slob, let
> > > > > alone slob + memcg.)
> > > >
> > > > Thanks for the clarification, it makes sense to use the byte interface
> > > > here for this, and thanks for pointing out that CONFIC_MEMCG_KMEM is
> > > > not part of CONFIG_MEMCG.
> > > >
> > > > One more question :) memcg kmem charging can still be disabled even
> > > > with !CONFIG_MEMCG_KMEM, right? In this case zswap charging will also
> > > > be off, which seems like an unintended side effect, right?
> > >
> > > memcg kmem charging can still be disabled even
> > > with CONFIG_MEMCG_KMEM***
> >
> > Yes, indeed, if the host is booted with the nokmem flag. Doing so will
> > turn off slab, percpu, and (as of recently) zswap.
> >
> > The zswap backing storage *is* kernel memory, so that seems like the
> > correct semantics for the flag.
>
> I honestly didn't consider it first as kernel memory, as the memory is
> coming from LRU pages. It can be confusing to think about, given that
> it is memory that the kernel allocates and manages, but it has user
> data. Anyway, maybe it's not worth overthinking this, since it can be
> considered as kernel memory.

Yup not worth overthinking this.

>
> >
> > That said, the distinction between kernel and user memory is becoming
> > increasingly odd. The more kernel memory we track, the more ridiculous
> > the size of the hole you punch into resource control by disabling it.
> >
> > Maybe we should just deprecate that knob altogether.
>
> Yeah a lot of used memory will just go unaccounted even with memcg
> enabled if the knob is disabled (which is what memcg is all about).
> This is even more significant now with zswap in the picture.

Yes, we should deprecate this knob. Last I remember there were users
facing zombie issues due to kernel memory for kernels before objcg
infrastructure and used this knob to resolve the zombie issue. So, I
think this can be deprecated. Though it would be safe to first warn
and deprecate in later release.