MIME-Version: 1.0
References: <20211222052457.1960701-1-shakeelb@google.com> <YcmaA9BS/DSB/iER@dhcp22.suse.cz>
In-Reply-To: <YcmaA9BS/DSB/iER@dhcp22.suse.cz>
From:   Shakeel Butt <shakeelb@google.com>
Date:   Thu, 30 Dec 2021 11:06:14 -0800
Message-ID: <CALvZod5ORV1y02TcATpRAQVObEPYNv0y0t49bS0f0NTvT6WRSQ@mail.gmail.com>
Subject: Re: [PATCH v2] memcg: add per-memcg vmalloc stat
To:     Michal Hocko <mhocko@suse.com>
Cc:     Johannes Weiner <hannes@cmpxchg.org>, Roman Gushchin <guro@fb.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux MM <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Michal,

On Mon, Dec 27, 2021 at 2:48 AM Michal Hocko <mhocko@suse.com> wrote:
>
[...]
> >       atomic_long_add(area->nr_pages, &nr_vmalloc_pages);
> > +     mod_memcg_page_state(area->pages[0], MEMCG_VMALLOC, area->nr_pages);
> >
> >       /*
> >        * If not enough pages were obtained to accomplish an
>
> Is it safe to assume that the whole area is always charged to the same
> memcg? I am not really deeply familiar with vmalloc internals but is it
> possible that an area could get resized/partially reused with a
> different charging context?

From what I understand, vmalloc areas are not resized or partially
reused at the moment. There is some ongoing discussion on caching it
but caching would also require updating the accounting part as well.

Regarding the whole area charged to the same memcg, the only way it
may get charged to different memcgs is if the process in which the
allocations are happening is migrated to a different memcg. We can
resolve this by traversing the pages in area->pages array (and use
lruvec based stats instead).

I did contemplate on making this a lruvec stat but decided to start
simple and if we ever need per-node stat then we can easily move to
lruvec based stats. Let me know what you think.

thanks,
Shakeel