linux-kernel - Re: [PATCH v1 2/4] mm: memcg: merge multiple page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 7 May 2024 17:26:34 -0700
From: "T.J. Mercier" <tjmercier@...gle.com>
To: Roman Gushchin <roman.gushchin@...ux.dev>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>, Andrew Morton <akpm@...ux-foundation.org>, 
	Muchun Song <muchun.song@...ux.dev>, Johannes Weiner <hannes@...xchg.org>, 
	Michal Hocko <mhocko@...nel.org>, Frank van der Linden <fvdl@...gle.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/4] mm: memcg: merge multiple page_counters into a
 single structure

On Fri, May 3, 2024 at 2:18 PM Roman Gushchin <roman.gushchin@...uxdev> wrote:
>
> On Fri, May 03, 2024 at 02:11:17PM -0700, Shakeel Butt wrote:
> > On Fri, May 03, 2024 at 01:18:33PM -0700, Roman Gushchin wrote:
> > [...]
> > >  enum mem_counter_type {
> > > +#ifdef CONFIG_MEMCG
> > >     MCT_MEMORY,             /* cgroup v1 and v2 */
> > >     MCT_SWAP,               /* cgroup v2 only */
> > >     MCT_MEMSW = MCT_SWAP,   /* cgroup v1 only */
> > >     MCT_KMEM,               /* cgroup v1 only */
> > >     MCT_TCPMEM,             /* cgroup v1 only */
> > > +#endif
> > > +#ifdef CONFIG_CGROUP_HUGETLB
> > > +   MCT_HUGETLB_MAX = __MCT_HUGETLB_MAX,
> > > +#endif
> > > +   __MCT_NR_ITEMS,
> > >  };
> > >
> >
> > Thanks for the awesome work. I haven't gone through all the patches yet
> > but wanted to ask a quick question. In the above enum are you trying to
> > do a union between memcg and hugetlb? It gave me a big pause to
> > understand what you are trying to do.
>
> Yep, sort of. So the page_counter structure supports N independent
> counters, where N is sufficient enough for both memcg and hugetlb cases.
>
> MCT_MEMORY, MCT_SWAP etc are used directly in the memcontrol.c code,
> while hugetlb code just indexes. MCT_HUGETLB_MAX magic is needed to define
> N at the compile time.

Where N is __MCT_NR_ITEMS for all the counter array lengths? That
doesn't look like it works if MCT_HUGETLB_MAX is small... i.e. there
is both CONFIG_MEMCG and CONFIG_CGROUP_HUGETLB and (__MCT_HUGETLB_MAX
= 1 or 3) since MCT_HUGETLB_MAX would be < MCT_TCPMEM and then
__MCT_NR_ITEMS would be wrong?

If so, what about:

enum mem_counter_type {
#ifdef CONFIG_MEMCG
    MCT_MEMORY, /* cgroup v1 and v2 */
    MCT_SWAP, /* cgroup v2 only */
    MCT_MEMSW = MCT_SWAP, /* cgroup v1 only */
    MCT_KMEM, /* cgroup v1 only */
    MCT_TCPMEM, /* cgroup v1 only */
#endif
    MCT_MEMCG_NR_ITEMS,
#ifdef CONFIG_CGROUP_HUGETLB
    MCT_HUGETLB_MAX = MCT_MEMCG_NR_ITEMS + __MCT_HUGETLB_MAX,
#else
    MCT_HUGETLB_MAX = 0,
#endif
    __MCT_NR_ITEMS = MAX(MCT_MEMCG_NR_ITEMS, MCT_HUGETLB_MAX)
};