Re: [PATCH 1/1] memcg/hugetlb: Adding hugeTLB counters to memory controller

[Joshua Hahn]

On Fri, Oct 18, 2024 at 5:34 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
>
> On Thu, Oct 17, 2024 at 09:04:38AM GMT, Joshua Hahn wrote:
> > HugeTLB is added as a metric in memcg_stat_item, and is updated in the
> > alloc and free methods for hugeTLB, after (un)charging has already been
> > committed. Changes are batched and updated / flushed like the rest of
> > the memcg stats, which makes additional overhead by the infrequent
> > hugetlb allocs / frees minimal.
> >
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
>
> I have an orthogonal cleanup request (i.e. after you are done with this
> work). Hugetlb is the last user of try-charge + commit protocol for
> memcg charging. I think we should just remove that and use a simple
> charge interface. You will need to reorder couple of things like
> allocating the folio first and then charge and you will need to do right
> cleanup on charge failing but I think it will cleanup the error path of
> alloc_hugetlb_folio() a lot.

That sounds good to me. I was originally planning to include the
hugeTLB accounting in the try charging mechanism (as to only include
it in memory.stat if it is also accounted for in memory.current. I will
think of another way to do this accounting so that cleanup becomes
easier once this patch is finished. One way I can think of is just to
check for the hugeTLB accounting config before adding the stats
and accounting for them.

Thank you for your feedback!

Joshua