linux-kernel - Re: [PATCH v3 1/1] memcg/hugetlb: Adding hugeTLB counters to memcg

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: <CAN+CAwORE8+P7Td9yebkb0TVT92SZv8oasrypewCspf5om1fYQ@mail.gmail.com>
Date: Fri, 1 Nov 2024 14:33:36 -0400
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Michal Hocko <mhocko@...e.com>, Johannes Weiner <hannes@...xchg.org>, nphamcs@...il.com, 
	shakeel.butt@...ux.dev, roman.gushchin@...ux.dev, muchun.song@...ux.dev, 
	tj@...nel.org, lizefan.x@...edance.com, mkoutny@...e.com, corbet@....net, 
	lnyng@...a.com, cgroups@...r.kernel.org, linux-mm@...ck.org, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH v3 1/1] memcg/hugetlb: Adding hugeTLB counters to memcg

On Thu, Oct 31, 2024 at 9:34 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Thu, 31 Oct 2024 15:03:34 -0400 Joshua Hahn <joshua.hahnjy@...il.com> wrote:
>
> > Andrew -- I am sorry to ask again, but do you think you can replace
> > the 3rd section in the patch (3. Implementation Details) with the
> > following paragraphs?
>
> No problem.
> : 3.  Implementation Details:
> : In the alloc / free hugetlb functions, we call lruvec_stat_mod_folio
> : regardless of whether memcg accounts hugetlb.  mem_cgroup_commit_charge
> : which is called from alloc_hugetlb_folio will set memcg for the folio
> : only if the CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING cgroup mount option is
> : used, so lruvec_stat_mod_folio accounts per-memcg hugetlb counters only
> : if the feature is enabled.  Regardless of whether memcg accounts for
> : hugetlb, the newly added global counter is updated and shown in
> : /proc/vmstat.
> :
> : The global counter is added because vmstats is the preferred framework
> : for cgroup stats.  It makes stat items consistent between global and
> : cgroups.  It also provides a per-node breakdown, which is useful.
> : Because it does not use cgroup-specific hooks, we also keep generic MM
> : code separate from memcg code.
> :
> : With this said, there are 2 problems:
> : (a) They are still not reported in memory.stat, which means the
> :     disparity between the memcg reports are still there.
> : (b) We cannot reasonably expect users to enable the hugeTLB controller
> :     just for the sake of hugeTLB usage reporting, especially since
> :     they don't have any use for hugeTLB usage enforcing [2].
> :
> : [1] https://lore.kernel.org/all/20231006184629.155543-1-nphamcs@gmail.com/
> : [2] Of course, we can't make a new patch for every feature that can be
> :     duplicated. However, since the existing solution of enabling the
> :     hugeTLB controller is an imperfect solution that still leaves a
> :     discrepancy between memory.stat and memory.curent, I think that it
> :     is reasonable to isolate the feature in this case.
>

Hello Andrew,

Thank you for your help as always. I apologize for not being clear in my
original request -- the "With this said, there are 2 problems:" paragraph is
part of the 2nd section (2. We already have a hugeTLB controller...) So the
outline will be:

This patch introduces...

1. Why is this patch necessary?\n
Currently, memcg hugeTLB accounting...

Aside from the consistency between...

2. We already have a hugeTLB controller. Why not use that?\n
It is true that hugeTLB tracks...

With this said, there are 2 problems:\n
  (a) They are still not...
  (b) We cannot reasonably...

3. Implementation Details\n
In the alloc / free hugetlb functions, ...

The global counter is added because...

[1] https://lore.kernel.org/ ...
[2] Of course, we can't make a new patch...

Thank you for your patience. I promise that this is the last change
to the patch message, I apologize for the frequent requests for
modifications. I hope you have a great day!
Joshua