linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: memcg: export workingset refault stats for cgroup v1
@ 2022-08-16 18:58 Yang Shi
  2022-08-16 22:06 ` Shakeel Butt
  2022-08-17 21:15 ` Shakeel Butt
  0 siblings, 2 replies; 7+ messages in thread
From: Yang Shi @ 2022-08-16 18:58 UTC (permalink / raw)
  To: hannes, mhocko, roman.gushchin, shakeelb, songmuchun, akpm
  Cc: shy828301, cgroups, linux-mm, linux-kernel

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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-16 18:58 [PATCH] mm: memcg: export workingset refault stats for cgroup v1 Yang Shi
@ 2022-08-16 22:06 ` Shakeel Butt
  2022-08-16 22:45   ` Yosry Ahmed
  2022-08-17  2:01   ` Yang Shi
  2022-08-17 21:15 ` Shakeel Butt
  1 sibling, 2 replies; 7+ messages in thread
From: Shakeel Butt @ 2022-08-16 22:06 UTC (permalink / raw)
  To: Yang Shi
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
	Andrew Morton, Cgroups, Linux MM, LKML, Yosry Ahmed

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.)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-16 22:06 ` Shakeel Butt
@ 2022-08-16 22:45   ` Yosry Ahmed
  2022-08-17  2:01   ` Yang Shi
  1 sibling, 0 replies; 7+ messages in thread
From: Yosry Ahmed @ 2022-08-16 22:45 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Yang Shi, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Muchun Song, Andrew Morton, Cgroups, Linux MM, LKML

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-16 22:06 ` Shakeel Butt
  2022-08-16 22:45   ` Yosry Ahmed
@ 2022-08-17  2:01   ` Yang Shi
  2022-08-17  2:05     ` Shakeel Butt
  1 sibling, 1 reply; 7+ messages in thread
From: Yang Shi @ 2022-08-17  2:01 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
	Andrew Morton, Cgroups, Linux MM, LKML, 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.

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-17  2:01   ` Yang Shi
@ 2022-08-17  2:05     ` Shakeel Butt
  2022-08-17 21:10       ` Yang Shi
  0 siblings, 1 reply; 7+ messages in thread
From: Shakeel Butt @ 2022-08-17  2:05 UTC (permalink / raw)
  To: Yang Shi
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
	Andrew Morton, Cgroups, Linux MM, LKML, Yosry Ahmed

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-17  2:05     ` Shakeel Butt
@ 2022-08-17 21:10       ` Yang Shi
  0 siblings, 0 replies; 7+ messages in thread
From: Yang Shi @ 2022-08-17 21:10 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
	Andrew Morton, Cgroups, Linux MM, LKML, Yosry Ahmed

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?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: memcg: export workingset refault stats for cgroup v1
  2022-08-16 18:58 [PATCH] mm: memcg: export workingset refault stats for cgroup v1 Yang Shi
  2022-08-16 22:06 ` Shakeel Butt
@ 2022-08-17 21:15 ` Shakeel Butt
  1 sibling, 0 replies; 7+ messages in thread
From: Shakeel Butt @ 2022-08-17 21:15 UTC (permalink / raw)
  To: Yang Shi
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
	Andrew Morton, Cgroups, Linux MM, LKML

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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-17 21:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 18:58 [PATCH] mm: memcg: export workingset refault stats for cgroup v1 Yang Shi
2022-08-16 22:06 ` Shakeel Butt
2022-08-16 22:45   ` Yosry Ahmed
2022-08-17  2:01   ` Yang Shi
2022-08-17  2:05     ` Shakeel Butt
2022-08-17 21:10       ` Yang Shi
2022-08-17 21:15 ` Shakeel Butt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).