linux-kernel - Re: [PATCH v2 1/2] hugetlb: memcg: account hugetlb-backed memory in memory controller

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]

Message-ID: <CAKEwX=M=8KYqvBTz9z1csrsFUpGf2tgWj-oyu96dSpRjn3ZnUQ@mail.gmail.com>
Date:   Thu, 28 Sep 2023 17:58:40 -0700
From:   Nhat Pham <nphamcs@...il.com>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     akpm@...ux-foundation.org, riel@...riel.com, hannes@...xchg.org,
        mhocko@...nel.org, roman.gushchin@...ux.dev, shakeelb@...gle.com,
        muchun.song@...ux.dev, tj@...nel.org, lizefan.x@...edance.com,
        shuah@...nel.org, mike.kravetz@...cle.com, linux-mm@...ck.org,
        kernel-team@...a.com, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org
Subject: Re: [PATCH v2 1/2] hugetlb: memcg: account hugetlb-backed memory in
 memory controller

On Thu, Sep 28, 2023 at 5:38 PM Yosry Ahmed <yosryahmed@...gle.com> wrote:
>
> <snip>
>
> >
> > +
> > +/**
> > + * mem_cgroup_hugetlb_charge_folio - Charge a newly allocated hugetlb folio.
> > + * @folio: folio to charge.
> > + * @gfp: reclaim mode
> > + *
> > + * This function charges an allocated hugetlbf folio to the memcg of the
> > + * current task.
> > + *
> > + * Returns 0 on success. Otherwise, an error code is returned.
> > + */
> > +int mem_cgroup_hugetlb_charge_folio(struct folio *folio, gfp_t gfp)
> > +{
> > +       struct mem_cgroup *memcg;
> > +       int ret;
> > +
> > +       if (mem_cgroup_disabled() ||
> > +               !(cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING))
>
> What happens if the memory controller is mounted in a cgroup v1
> hierarchy? It appears to me that we *will* go through with hugetlb
> charging in this case?

Ah right, cgroup v1. Does it not work with mount flag guarding?
What's the behavior of cgroup v1 when it comes to memory
recursive protection for e.g (which this mount flag is based on)?

If it doesn't work, we'll have to add a separate knob for v1 -
no biggies.

Other than this concern, I don't have anything against cgroup v1
having this feature per se - everything should still work. But let
I know if it can break cgroupv1 accounting otherwise :)

>
> >
> > +               return 0;
> > +
> > +       memcg = get_mem_cgroup_from_current();
> > +       ret = charge_memcg(folio, memcg, gfp);
> > +       mem_cgroup_put(memcg);
> > +
> > +       return ret;
> > +}
> > +
> >  /**
> >   * mem_cgroup_swapin_charge_folio - Charge a newly allocated folio for swapin.
> >   * @folio: folio to charge.
> > --
> > 2.34.1