linux-kernel - Re: [PATCH v9 3/8] hugetlb_cgroup: add reservation accounting for private mappings

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] [day] [month] [year] [list]

Date:   Wed, 22 Jan 2020 13:40:53 -0800
From:   Mina Almasry <almasrymina@...gle.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     shuah <shuah@...nel.org>, David Rientjes <rientjes@...gle.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Greg Thelen <gthelen@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        open list <linux-kernel@...r.kernel.org>, linux-mm@...ck.org,
        linux-kselftest@...r.kernel.org, cgroups@...r.kernel.org,
        Aneesh Kumar <aneesh.kumar@...ux.vnet.ibm.com>,
        Michal Koutný <mkoutny@...e.com>,
        Hillf Danton <hdanton@...a.com>
Subject: Re: [PATCH v9 3/8] hugetlb_cgroup: add reservation accounting for
 private mappings

On Fri, Jan 17, 2020 at 2:09 PM Mike Kravetz <mike.kravetz@...cle.com> wrote:
>
> On 1/14/20 2:52 PM, Mina Almasry wrote:
> > On Mon, Jan 13, 2020 at 4:55 PM Mike Kravetz <mike.kravetz@...cle.com> wrote:
> >>> +#ifdef CONFIG_CGROUP_HUGETLB
> >>> +     /*
> >>> +      * Since we check for HPAGE_RESV_OWNER above, this must a private
> >>> +      * mapping, and these values should be none-zero, and should point to
> >>> +      * the hugetlb_cgroup counter to uncharge for this reservation.
> >>> +      */
> >>> +     WARN_ON(!resv->reservation_counter);
> >>> +     WARN_ON(!resv->pages_per_hpage);
> >>> +     WARN_ON(!resv->css);
> >>
> >> I was once again wondering if these were always non-NULL for private mappings.
> >> It seems that reservation_counter (h_gc) would be NULL in these cases from
> >> these early checks in hugetlb_cgroup_charge_cgroup().
> >>
> >
> > You are right. I'm fixing in v10 the code and comments to account for
> > h_cg potentially being NULL, but I'm having trouble testing. Looking
> > at the code, I'm a bit confused by the checks. Seems to me
> > hugetlb_cgroup_disabled() is the same as #ifdef CONFIG_CGROUP_HUGETLB;
> > I can't find a way to enable the Kconfig but have that return false
> > unless I hack the code.
>
> What about the boot options?
>
> cgroup_disable=
> cgroup_no_v1=

Thanks, cgroup_disable=hugetlb does it. I ran the the libhugetlbfs
tests with patchset v10 and it passed, so it seems the latest version
of the patch should be fine. Of course my hugetlb cgroup tests fail
outright when hugetlb cgroups are disabled so those don't say anything
useful.