Re: [PATCH 2/2] vmalloc: Account memcg per vmalloc

[Matthew Wilcox <willy@infradead.org>]

On Wed, Dec 11, 2024 at 12:58:36PM -0800, Shakeel Butt wrote:
> On Wed, Dec 11, 2024 at 08:20:36PM +0000, Matthew Wilcox wrote:
> > Umm, I don't think you know which vmalloc allocation a page came from
> > today?  I've sent patches to add that information before, but they were
> > rejected. 
> 
> Do you have a link handy for that discussion?

It's not really relevant any more ...

https://lore.kernel.org/linux-mm/20180518194519.3820-18-willy@infradead.org/

and subsequent discussion:

https://lore.kernel.org/linux-mm/20180611121129.GB12912@bombadil.infradead.org/

It all predates memdesc.

> Yes you are correct. At the moment it is a guesswork and exhaustive
> search into multiple sources.

At least I should be able to narrow it down somewhat if we have a
PGTY_vmalloc.

> > I actually want to improve this, without adding additional overhead.
> > What I'm working on right now (before I got waylaid by this bug) is:
> > 
> > +struct choir {
> > +       struct kref refcount;
> > +       unsigned int nr;
> > +       struct page *pages[] __counted_by(nr);
> > +};
> > 
> > and rewriting vmalloc to be based on choirs instead of its own pages.
> > One thing I've come to realise today is that the obj_cgroup pointer
> > needs to be in the choir and not in the vm_struct so that we uncharge the
> > allocation when the choir refcount drops to 0, not when the allocation
> > is unmapped.
> 
> What/who else can take a reference on a choir?

The first user is remap_vmalloc_range() which today takes a
mapcount+refcount on the page underlying the vmalloc inside
vm_insert_page().

But I see choirs being useful more widely; for example in the XFS buffer
cache (which wouldn't be mappable to userspace).  They might even be
the right approach for zswap, replacing zpdesc.  Haven't looked into
that in enough detail yet.