[PATCH] mm: memcg: export workingset refault stats for cgroup v1

[PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Yang Shi]

Workingset refault stats are important and usefule metrics to measure
how well reclaimer and swapping work and how healthy the services are,
but they are just available for cgroup v2.  There are still plenty users
with cgroup v1, export the stats for cgroup v1.

Signed-off-by: Yang Shi <shy828301@gmail.com>
---
I do understand the development of cgroup v1 is actually stalled and
the community is reluctant to accept new features for v1.  However
the workingset refault stats are really quite useful and exporting
two new stats, which have been supported by v2, seems ok IMHO.  So
hopefully this patch could be considered.  Thanks.

 mm/memcontrol.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b69979c9ced5..e300437896dc 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3975,6 +3975,8 @@ static const unsigned int memcg1_stats[] = {
 	NR_FILE_MAPPED,
 	NR_FILE_DIRTY,
 	NR_WRITEBACK,
+	WORKINGSET_REFAULT_ANON,
+	WORKINGSET_REFAULT_FILE,
 	MEMCG_SWAP,
 };
 
@@ -3988,6 +3990,8 @@ static const char *const memcg1_stat_names[] = {
 	"mapped_file",
 	"dirty",
 	"writeback",
+	"workingset_refault_anon",
+	"workingset_refault_file",
 	"swap",
 };
 
@@ -4016,7 +4020,8 @@ static int memcg_stat_show(struct seq_file *m, void *v)
 		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
 			continue;
 		nr = memcg_page_state_local(memcg, memcg1_stats[i]);
-		seq_printf(m, "%s %lu\n", memcg1_stat_names[i], nr * PAGE_SIZE);
+		seq_printf(m, "%s %lu\n", memcg1_stat_names[i],
+			   nr * memcg_page_state_unit(memcg1_stats[i]));
 	}
 
 	for (i = 0; i < ARRAY_SIZE(memcg1_events); i++)
@@ -4047,7 +4052,7 @@ static int memcg_stat_show(struct seq_file *m, void *v)
 			continue;
 		nr = memcg_page_state(memcg, memcg1_stats[i]);
 		seq_printf(m, "total_%s %llu\n", memcg1_stat_names[i],
-						(u64)nr * PAGE_SIZE);
+			   (u64)nr * memcg_page_state_unit(memcg1_stats[i]));
 	}
 
 	for (i = 0; i < ARRAY_SIZE(memcg1_events); i++)
-- 
2.26.3

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Shakeel Butt]

On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
>
> Workingset refault stats are important and usefule metrics to measure
> how well reclaimer and swapping work and how healthy the services are,
> but they are just available for cgroup v2.  There are still plenty users
> with cgroup v1, export the stats for cgroup v1.
>
> Signed-off-by: Yang Shi <shy828301@gmail.com>
> ---
> I do understand the development of cgroup v1 is actually stalled and
> the community is reluctant to accept new features for v1.  However
> the workingset refault stats are really quite useful and exporting
> two new stats, which have been supported by v2, seems ok IMHO.  So
> hopefully this patch could be considered.  Thanks.
>

Is just workingset refault good enough for your use-case? What about
the other workingset stats? I don't have a strong opinion against
adding these to v1 and I think these specific stats should be fine.
(There is subtlety in exposing objcg based stats (i.e. reparenting) in
v1 due to non-hierarchical stats in v1. I remember Yosry and Muchun
were looking into that.)

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Yosry Ahmed]

On Tue, Aug 16, 2022 at 3:06 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
> >
> > Workingset refault stats are important and usefule metrics to measure
> > how well reclaimer and swapping work and how healthy the services are,
> > but they are just available for cgroup v2.  There are still plenty users
> > with cgroup v1, export the stats for cgroup v1.
> >
> > Signed-off-by: Yang Shi <shy828301@gmail.com>
> > ---
> > I do understand the development of cgroup v1 is actually stalled and
> > the community is reluctant to accept new features for v1.  However
> > the workingset refault stats are really quite useful and exporting
> > two new stats, which have been supported by v2, seems ok IMHO.  So
> > hopefully this patch could be considered.  Thanks.
> >
>
> Is just workingset refault good enough for your use-case? What about
> the other workingset stats? I don't have a strong opinion against
> adding these to v1 and I think these specific stats should be fine.
> (There is subtlety in exposing objcg based stats (i.e. reparenting) in
> v1 due to non-hierarchical stats in v1. I remember Yosry and Muchun
> were looking into that.)

I think only kernel memory stats and zswap stats are objcg-based at
this point, right? The workingset refault stats seem to be memcg-based
and should not face the reparenting problem.

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Yang Shi]

On Tue, Aug 16, 2022 at 3:06 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
> >
> > Workingset refault stats are important and usefule metrics to measure
> > how well reclaimer and swapping work and how healthy the services are,
> > but they are just available for cgroup v2.  There are still plenty users
> > with cgroup v1, export the stats for cgroup v1.
> >
> > Signed-off-by: Yang Shi <shy828301@gmail.com>
> > ---
> > I do understand the development of cgroup v1 is actually stalled and
> > the community is reluctant to accept new features for v1.  However
> > the workingset refault stats are really quite useful and exporting
> > two new stats, which have been supported by v2, seems ok IMHO.  So
> > hopefully this patch could be considered.  Thanks.
> >
>
> Is just workingset refault good enough for your use-case? What about
> the other workingset stats? I don't have a strong opinion against
> adding these to v1 and I think these specific stats should be fine.

The workingset refault is good enough for our usercase, but I don't
mind adding all the workingset_* stats if nobody has objection.

> (There is subtlety in exposing objcg based stats (i.e. reparenting) in
> v1 due to non-hierarchical stats in v1. I remember Yosry and Muchun
> were looking into that.)

The workingset_* stats should have nothing to do with obj based stats IIUC.

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Shakeel Butt]

On Tue, Aug 16, 2022 at 7:01 PM Yang Shi <shy828301@gmail.com> wrote:
>
> On Tue, Aug 16, 2022 at 3:06 PM Shakeel Butt <shakeelb@google.com> wrote:
> >
> > On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
> > >
> > > Workingset refault stats are important and usefule metrics to measure
> > > how well reclaimer and swapping work and how healthy the services are,
> > > but they are just available for cgroup v2.  There are still plenty users
> > > with cgroup v1, export the stats for cgroup v1.
> > >
> > > Signed-off-by: Yang Shi <shy828301@gmail.com>
> > > ---
> > > I do understand the development of cgroup v1 is actually stalled and
> > > the community is reluctant to accept new features for v1.  However
> > > the workingset refault stats are really quite useful and exporting
> > > two new stats, which have been supported by v2, seems ok IMHO.  So
> > > hopefully this patch could be considered.  Thanks.
> > >
> >
> > Is just workingset refault good enough for your use-case? What about
> > the other workingset stats? I don't have a strong opinion against
> > adding these to v1 and I think these specific stats should be fine.
>
> The workingset refault is good enough for our usercase, but I don't
> mind adding all the workingset_* stats if nobody has objection.

For now let's just start with what your use-case needs. If in future
there is a need we can add other workingset_* stats as well.

>
> > (There is subtlety in exposing objcg based stats (i.e. reparenting) in
> > v1 due to non-hierarchical stats in v1. I remember Yosry and Muchun
> > were looking into that.)
>
> The workingset_* stats should have nothing to do with obj based stats IIUC.

Yeah, that was just FYI for anyone in future who wants to export such
stat in v1.

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Yang Shi]

On Tue, Aug 16, 2022 at 7:05 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Tue, Aug 16, 2022 at 7:01 PM Yang Shi <shy828301@gmail.com> wrote:
> >
> > On Tue, Aug 16, 2022 at 3:06 PM Shakeel Butt <shakeelb@google.com> wrote:
> > >
> > > On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
> > > >
> > > > Workingset refault stats are important and usefule metrics to measure
> > > > how well reclaimer and swapping work and how healthy the services are,
> > > > but they are just available for cgroup v2.  There are still plenty users
> > > > with cgroup v1, export the stats for cgroup v1.
> > > >
> > > > Signed-off-by: Yang Shi <shy828301@gmail.com>
> > > > ---
> > > > I do understand the development of cgroup v1 is actually stalled and
> > > > the community is reluctant to accept new features for v1.  However
> > > > the workingset refault stats are really quite useful and exporting
> > > > two new stats, which have been supported by v2, seems ok IMHO.  So
> > > > hopefully this patch could be considered.  Thanks.
> > > >
> > >
> > > Is just workingset refault good enough for your use-case? What about
> > > the other workingset stats? I don't have a strong opinion against
> > > adding these to v1 and I think these specific stats should be fine.
> >
> > The workingset refault is good enough for our usercase, but I don't
> > mind adding all the workingset_* stats if nobody has objection.
>
> For now let's just start with what your use-case needs. If in future
> there is a need we can add other workingset_* stats as well.

Sure, works for me.

>
> >
> > > (There is subtlety in exposing objcg based stats (i.e. reparenting) in
> > > v1 due to non-hierarchical stats in v1. I remember Yosry and Muchun
> > > were looking into that.)
> >
> > The workingset_* stats should have nothing to do with obj based stats IIUC.
>
> Yeah, that was just FYI for anyone in future who wants to export such
> stat in v1.

Thanks, Shakeel. If it looks good to me, would you please ack the patch?

Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1

[Shakeel Butt]

On Tue, Aug 16, 2022 at 11:58 AM Yang Shi <shy828301@gmail.com> wrote:
>
> Workingset refault stats are important and usefule metrics to measure

*useful

> how well reclaimer and swapping work and how healthy the services are,
> but they are just available for cgroup v2.  There are still plenty users
> with cgroup v1, export the stats for cgroup v1.
>
> Signed-off-by: Yang Shi <shy828301@gmail.com>

Acked-by: Shakeel Butt <shakeelb@google.com>