Re: [External] Re: [PATCH v5 2/7] mm: memcontrol: convert NR_ANON_THPS account to pages

[Muchun Song]

On Thu, Dec 24, 2020 at 6:08 AM Shakeel Butt <shakeelb@xxxxxxxxxx> wrote:
>
> On Wed, Dec 16, 2020 at 7:45 PM Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
> >
> > Currently we use struct per_cpu_nodestat to cache the vmstat
> > counters, which leads to inaccurate statistics expecially THP
>
> *especially

Thanks.

>
> > vmstat counters. In the systems with hundreads of processors
>
> *hundreds

Thanks.

>
> > it can be GBs of memory. For example, for a 96 CPUs system,
> > the threshold is the maximum number of 125. And the per cpu
> > counters can cache 23.4375 GB in total.
> >
> > The THP page is already a form of batched addition (it will
> > add 512 worth of memory in one go) so skipping the batching
> > seems like sensible. Although every THP stats update overflows
> > the per-cpu counter, resorting to atomic global updates. But
> > it can make the statistics more accuracy for the THP vmstat
> > counters.
> >
> > So we convert the NR_ANON_THPS account to pages. This patch
> > is consistent with 8f182270dfec ("mm/swap.c: flush lru pvecs
> > on compound page arrival"). Doing this also can make the unit
> > of vmstat counters more unified. Finally, the unit of the vmstat
> > counters are pages, kB and bytes. The B/KB suffix can tell us
> > that the unit is bytes or kB. The rest which is without suffix
> > are pages.
> >
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
>
> I agree with the motivation behind this patch but I would like to see
> some performance numbers in the commit message. We might agree to pay
> the price but at least we will know what exactly that cost is.

Do you have any recommendations about benchmarks?
I can do a test. Thanks very much.

-- 
Yours,
Muchun