[PATCH v2 0/2] Expose swapcache stat for memcg v1

[PATCH v2 0/2] Expose swapcache stat for memcg v1

[Liu Shixin]

The first patch expose swapcache stat for memcg v1, the second patch
remote unused do_memsw_account() in memcg1_stat_format().

v1->v2: Cover MEMCG_SWAP with CONFIG_SWAP and delete unused code suggested
	by Michal.

Liu Shixin (2):
  memcg: expose swapcache stat for memcg v1
  memcg: remove unused do_memsw_account in memcg1_stat_format

 Documentation/admin-guide/cgroup-v1/memory.rst |  1 +
 mm/memcontrol.c                                | 15 ++++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] memcg: expose swapcache stat for memcg v1

[Liu Shixin]

Since commit b6038942480e ("mm: memcg: add swapcache stat for memcg v2")
adds swapcache stat for the cgroup v2, it seems there is no reason to
hide it in memcg v1. Conversely, with swapcached it is more accurate to
evaluate the available memory for memcg.

Suggested-by: Yosry Ahmed <yosryahmed@google.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 Documentation/admin-guide/cgroup-v1/memory.rst | 1 +
 mm/memcontrol.c                                | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst
index fabaad3fd9c2..fb4abe0dc228 100644
--- a/Documentation/admin-guide/cgroup-v1/memory.rst
+++ b/Documentation/admin-guide/cgroup-v1/memory.rst
@@ -546,6 +546,7 @@ memory.stat file includes following statistics:
                     event happens each time a page is unaccounted from the
                     cgroup.
     swap            # of bytes of swap usage
+    swapcached      # of bytes of swap cached in memory
     dirty           # of bytes that are waiting to get written back to the disk.
     writeback       # of bytes of file/anon cache that are queued for syncing to
                     disk.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c465829db92b..78ea10c5a636 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4067,7 +4067,10 @@ static const unsigned int memcg1_stats[] = {
 	NR_WRITEBACK,
 	WORKINGSET_REFAULT_ANON,
 	WORKINGSET_REFAULT_FILE,
+#ifdef CONFIG_SWAP
 	MEMCG_SWAP,
+	NR_SWAPCACHE,
+#endif
 };
 
 static const char *const memcg1_stat_names[] = {
@@ -4082,7 +4085,10 @@ static const char *const memcg1_stat_names[] = {
 	"writeback",
 	"workingset_refault_anon",
 	"workingset_refault_file",
+#ifdef CONFIG_SWAP
 	"swap",
+	"swapcached",
+#endif
 };
 
 /* Universal VM events cgroup1 shows, original sort order */
-- 
2.25.1

[PATCH v2 2/2] memcg: remove unused do_memsw_account in memcg1_stat_format

[Liu Shixin]

Since commit b25806dcd3d5("mm: memcontrol: deprecate swapaccounting=0 mode")
do_memsw_account() is synonymous with !cgroup_subsys_on_dfl(memory_cgrp_subsys),
It always equals true in memcg1_stat_format(). Remove the unused code.

Suggested-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/memcontrol.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 78ea10c5a636..bb9a617be046 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4112,8 +4112,6 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
 	for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
 		unsigned long nr;
 
-		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
-			continue;
 		nr = memcg_page_state_local(memcg, memcg1_stats[i]);
 		seq_buf_printf(s, "%s %lu\n", memcg1_stat_names[i],
 			   nr * memcg_page_state_unit(memcg1_stats[i]));
@@ -4136,15 +4134,12 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
 	}
 	seq_buf_printf(s, "hierarchical_memory_limit %llu\n",
 		       (u64)memory * PAGE_SIZE);
-	if (do_memsw_account())
-		seq_buf_printf(s, "hierarchical_memsw_limit %llu\n",
-			       (u64)memsw * PAGE_SIZE);
+	seq_buf_printf(s, "hierarchical_memsw_limit %llu\n",
+		       (u64)memsw * PAGE_SIZE);
 
 	for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
 		unsigned long nr;
 
-		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
-			continue;
 		nr = memcg_page_state(memcg, memcg1_stats[i]);
 		seq_buf_printf(s, "total_%s %llu\n", memcg1_stat_names[i],
 			   (u64)nr * memcg_page_state_unit(memcg1_stats[i]));
-- 
2.25.1

Re: [PATCH v2 0/2] Expose swapcache stat for memcg v1

[Tejun Heo]

On Fri, Sep 15, 2023 at 06:58:43PM +0800, Liu Shixin wrote:
> The first patch expose swapcache stat for memcg v1, the second patch
> remote unused do_memsw_account() in memcg1_stat_format().
> 
> v1->v2: Cover MEMCG_SWAP with CONFIG_SWAP and delete unused code suggested
> 	by Michal.
> 
> Liu Shixin (2):
>   memcg: expose swapcache stat for memcg v1
>   memcg: remove unused do_memsw_account in memcg1_stat_format

Acked-by: Tejun heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH v2 2/2] memcg: remove unused do_memsw_account in memcg1_stat_format

[Yosry Ahmed]

On Fri, Sep 15, 2023 at 3:05 AM Liu Shixin <liushixin2@huawei.com> wrote:
>
> Since commit b25806dcd3d5("mm: memcontrol: deprecate swapaccounting=0 mode")
> do_memsw_account() is synonymous with !cgroup_subsys_on_dfl(memory_cgrp_subsys),
> It always equals true in memcg1_stat_format(). Remove the unused code.
>
> Suggested-by: Michal Koutný <mkoutny@suse.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Reviewed-by: Yosry Ahmed <yosryahmed@google.com>

> ---
>  mm/memcontrol.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 78ea10c5a636..bb9a617be046 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4112,8 +4112,6 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
>         for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
>                 unsigned long nr;
>
> -               if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
> -                       continue;
>                 nr = memcg_page_state_local(memcg, memcg1_stats[i]);
>                 seq_buf_printf(s, "%s %lu\n", memcg1_stat_names[i],
>                            nr * memcg_page_state_unit(memcg1_stats[i]));
> @@ -4136,15 +4134,12 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
>         }
>         seq_buf_printf(s, "hierarchical_memory_limit %llu\n",
>                        (u64)memory * PAGE_SIZE);
> -       if (do_memsw_account())
> -               seq_buf_printf(s, "hierarchical_memsw_limit %llu\n",
> -                              (u64)memsw * PAGE_SIZE);
> +       seq_buf_printf(s, "hierarchical_memsw_limit %llu\n",
> +                      (u64)memsw * PAGE_SIZE);
>
>         for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) {
>                 unsigned long nr;
>
> -               if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
> -                       continue;
>                 nr = memcg_page_state(memcg, memcg1_stats[i]);
>                 seq_buf_printf(s, "total_%s %llu\n", memcg1_stat_names[i],
>                            (u64)nr * memcg_page_state_unit(memcg1_stats[i]));
> --
> 2.25.1
>