On Fri, Jan 17, 2020 at 2:09 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > > On 1/14/20 2:52 PM, Mina Almasry wrote: > > On Mon, Jan 13, 2020 at 4:55 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> 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.
