All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] memcg: add per-memcg total kernel memory stat
@ 2022-02-01 20:08 Yosry Ahmed
  2022-02-01 20:26 ` Shakeel Butt
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Yosry Ahmed @ 2022-02-01 20:08 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Muchun Song, Shakeel Butt
  Cc: linux-kernel, linux-mm, Yosry Ahmed

Currently memcg stats show several types of kernel memory:
kernel stack, page tables, sock, vmalloc, and slab.
However, there are other allocations with __GFP_ACCOUNT
(or supersets such as GFP_KERNEL_ACCOUNT) that are not accounted
in any of those stats, a few examples are:
- various kvm allocations (e.g. allocated pages to create vcpus)
- io_uring
- tmp_page in pipes during pipe_write()
- bpf ringbuffers
- unix sockets

Keeping track of the total kernel memory is essential for the ease of
migration from cgroup v1 to v2 as there are large discrepancies between
v1's kmem.usage_in_bytes and the sum of the available kernel memory stats
in v2. Adding separate memcg stats for all __GFP_ACCOUNT kernel
allocations is an impractical maintenance burden as there a lot of those
all over the kernel code, with more use cases likely to show up in the
future.

Therefore, add a "kernel" memcg stat that is analogous to kmem
page counter, with added benefits such as using rstat infrastructure
which aggregates stats more efficiently. Additionally, this provides a
lighter alternative in case the legacy kmem is deprecated in the future

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
 Documentation/admin-guide/cgroup-v2.rst |  5 +++++
 include/linux/memcontrol.h              |  1 +
 mm/memcontrol.c                         | 24 ++++++++++++++++++------
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 5aa368d165da..a0027d570a7f 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1317,6 +1317,11 @@ PAGE_SIZE multiple when read back.
 	  vmalloc (npn)
 		Amount of memory used for vmap backed memory.
 
+	  kernel (npn)
+		Amount of total kernel memory, including
+		(kernel_stack, pagetables, percpu, vmalloc, slab) in
+		addition to other kernel memory use cases.
+
 	  shmem
 		Amount of cached filesystem data that is swap-backed,
 		such as tmpfs, shm segments, shared anonymous mmap()s
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index b72d75141e12..fa51986365a4 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -34,6 +34,7 @@ enum memcg_stat_item {
 	MEMCG_SOCK,
 	MEMCG_PERCPU_B,
 	MEMCG_VMALLOC,
+	MEMCG_KMEM,
 	MEMCG_NR_STAT,
 };
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 09d342c7cbd0..c55d7056ac98 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1376,6 +1376,7 @@ static const struct memory_stat memory_stats[] = {
 	{ "percpu",			MEMCG_PERCPU_B			},
 	{ "sock",			MEMCG_SOCK			},
 	{ "vmalloc",			MEMCG_VMALLOC			},
+	{ "kernel",			MEMCG_KMEM			},
 	{ "shmem",			NR_SHMEM			},
 	{ "file_mapped",		NR_FILE_MAPPED			},
 	{ "file_dirty",			NR_FILE_DIRTY			},
@@ -2979,6 +2980,19 @@ static void memcg_free_cache_id(int id)
 	ida_simple_remove(&memcg_cache_ida, id);
 }
 
+static void mem_cgroup_kmem_record(struct mem_cgroup *memcg,
+				   int nr_pages)
+{
+	mod_memcg_state(memcg, MEMCG_KMEM, nr_pages);
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
+		if (nr_pages > 0)
+			page_counter_charge(&memcg->kmem, nr_pages);
+		else
+			page_counter_uncharge(&memcg->kmem, -nr_pages);
+	}
+}
+
+
 /*
  * obj_cgroup_uncharge_pages: uncharge a number of kernel pages from a objcg
  * @objcg: object cgroup to uncharge
@@ -2991,8 +3005,7 @@ static void obj_cgroup_uncharge_pages(struct obj_cgroup *objcg,
 
 	memcg = get_mem_cgroup_from_objcg(objcg);
 
-	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
-		page_counter_uncharge(&memcg->kmem, nr_pages);
+	mem_cgroup_kmem_record(memcg, -nr_pages);
 	refill_stock(memcg, nr_pages);
 
 	css_put(&memcg->css);
@@ -3018,8 +3031,7 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp,
 	if (ret)
 		goto out;
 
-	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
-		page_counter_charge(&memcg->kmem, nr_pages);
+	mem_cgroup_kmem_record(memcg, nr_pages);
 out:
 	css_put(&memcg->css);
 
@@ -6801,8 +6813,8 @@ static void uncharge_batch(const struct uncharge_gather *ug)
 		page_counter_uncharge(&ug->memcg->memory, ug->nr_memory);
 		if (do_memsw_account())
 			page_counter_uncharge(&ug->memcg->memsw, ug->nr_memory);
-		if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && ug->nr_kmem)
-			page_counter_uncharge(&ug->memcg->kmem, ug->nr_kmem);
+		if (ug->nr_kmem)
+			mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
 		memcg_oom_recover(ug->memcg);
 	}
 
-- 
2.35.0.rc2.247.g8bbb082509-goog


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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 20:08 [PATCH] memcg: add per-memcg total kernel memory stat Yosry Ahmed
@ 2022-02-01 20:26 ` Shakeel Butt
  2022-02-01 21:00 ` Johannes Weiner
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Shakeel Butt @ 2022-02-01 20:26 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Muchun Song, LKML,
	Linux MM

On Tue, Feb 1, 2022 at 12:08 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> Currently memcg stats show several types of kernel memory:
> kernel stack, page tables, sock, vmalloc, and slab.
> However, there are other allocations with __GFP_ACCOUNT
> (or supersets such as GFP_KERNEL_ACCOUNT) that are not accounted
> in any of those stats, a few examples are:
> - various kvm allocations (e.g. allocated pages to create vcpus)
> - io_uring
> - tmp_page in pipes during pipe_write()
> - bpf ringbuffers
> - unix sockets
>
> Keeping track of the total kernel memory is essential for the ease of
> migration from cgroup v1 to v2 as there are large discrepancies between
> v1's kmem.usage_in_bytes and the sum of the available kernel memory stats
> in v2. Adding separate memcg stats for all __GFP_ACCOUNT kernel
> allocations is an impractical maintenance burden as there a lot of those
> all over the kernel code, with more use cases likely to show up in the
> future.
>
> Therefore, add a "kernel" memcg stat that is analogous to kmem
> page counter, with added benefits such as using rstat infrastructure
> which aggregates stats more efficiently. Additionally, this provides a
> lighter alternative in case the legacy kmem is deprecated in the future
>
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>

Thanks Yosry. Just to emphasize further, in our gradual migration to
v2 (exposing v2 interfaces in v1 and removing v1-only interfaces), the
difference between kernel memory from v1 and v2 is very prominent for
some workloads. This patch will definitely ease the v2 migration.

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

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 20:08 [PATCH] memcg: add per-memcg total kernel memory stat Yosry Ahmed
  2022-02-01 20:26 ` Shakeel Butt
@ 2022-02-01 21:00 ` Johannes Weiner
  2022-02-01 22:01   ` Yosry Ahmed
  2022-02-01 22:10   ` Yosry Ahmed
  2022-02-02  0:47 ` kernel test robot
  2022-02-02  5:41   ` kernel test robot
  3 siblings, 2 replies; 8+ messages in thread
From: Johannes Weiner @ 2022-02-01 21:00 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Andrew Morton, Michal Hocko, Muchun Song, Shakeel Butt,
	linux-kernel, linux-mm

Hello Yosry,

On Tue, Feb 01, 2022 at 08:08:23PM +0000, Yosry Ahmed wrote:
> Currently memcg stats show several types of kernel memory:
> kernel stack, page tables, sock, vmalloc, and slab.
> However, there are other allocations with __GFP_ACCOUNT
> (or supersets such as GFP_KERNEL_ACCOUNT) that are not accounted
> in any of those stats, a few examples are:
> - various kvm allocations (e.g. allocated pages to create vcpus)
> - io_uring
> - tmp_page in pipes during pipe_write()
> - bpf ringbuffers
> - unix sockets
> 
> Keeping track of the total kernel memory is essential for the ease of
> migration from cgroup v1 to v2 as there are large discrepancies between
> v1's kmem.usage_in_bytes and the sum of the available kernel memory stats
> in v2. Adding separate memcg stats for all __GFP_ACCOUNT kernel
> allocations is an impractical maintenance burden as there a lot of those
> all over the kernel code, with more use cases likely to show up in the
> future.

No objection, I'm just curious how it makes migration to v2 easier in
particular. Or is it just that you've used the v1 stat to track
application regressions and would like to continue doing that in v2?

> Therefore, add a "kernel" memcg stat that is analogous to kmem
> page counter, with added benefits such as using rstat infrastructure
> which aggregates stats more efficiently. Additionally, this provides a
> lighter alternative in case the legacy kmem is deprecated in the future
> 
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> ---
>  Documentation/admin-guide/cgroup-v2.rst |  5 +++++
>  include/linux/memcontrol.h              |  1 +
>  mm/memcontrol.c                         | 24 ++++++++++++++++++------
>  3 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index 5aa368d165da..a0027d570a7f 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -1317,6 +1317,11 @@ PAGE_SIZE multiple when read back.
>  	  vmalloc (npn)
>  		Amount of memory used for vmap backed memory.
>  
> +	  kernel (npn)
> +		Amount of total kernel memory, including
> +		(kernel_stack, pagetables, percpu, vmalloc, slab) in
> +		addition to other kernel memory use cases.
> +
>  	  shmem
>  		Amount of cached filesystem data that is swap-backed,
>  		such as tmpfs, shm segments, shared anonymous mmap()s
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index b72d75141e12..fa51986365a4 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -34,6 +34,7 @@ enum memcg_stat_item {
>  	MEMCG_SOCK,
>  	MEMCG_PERCPU_B,
>  	MEMCG_VMALLOC,
> +	MEMCG_KMEM,
>  	MEMCG_NR_STAT,
>  };
>  
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 09d342c7cbd0..c55d7056ac98 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1376,6 +1376,7 @@ static const struct memory_stat memory_stats[] = {
>  	{ "percpu",			MEMCG_PERCPU_B			},
>  	{ "sock",			MEMCG_SOCK			},
>  	{ "vmalloc",			MEMCG_VMALLOC			},
> +	{ "kernel",			MEMCG_KMEM			},

It's a superset of percpu, sock, vmalloc etc., so please move it ahead
of them.

	anon
	file
	kernel
	kernel_stack
	pagetables
	...

and in the doc as well.

>  	{ "shmem",			NR_SHMEM			},
>  	{ "file_mapped",		NR_FILE_MAPPED			},
>  	{ "file_dirty",			NR_FILE_DIRTY			},
> @@ -2979,6 +2980,19 @@ static void memcg_free_cache_id(int id)
>  	ida_simple_remove(&memcg_cache_ida, id);
>  }
>  
> +static void mem_cgroup_kmem_record(struct mem_cgroup *memcg,
> +				   int nr_pages)

No real need for the namespace prefix since it's a static
function. How about account_kmem()? Avoids the line wrap, too.

Otherwise, looks good to me, so with those changes:

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks!

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 21:00 ` Johannes Weiner
@ 2022-02-01 22:01   ` Yosry Ahmed
  2022-02-01 22:10   ` Yosry Ahmed
  1 sibling, 0 replies; 8+ messages in thread
From: Yosry Ahmed @ 2022-02-01 22:01 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Michal Hocko, Muchun Song, Shakeel Butt,
	linux-kernel, linux-mm

[-- Attachment #1: Type: text/plain, Size: 5138 bytes --]

Hello Johannes,

Thanks so much for your review.

On Tue, Feb 1, 2022 at 1:00 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> Hello Yosry,
>
> On Tue, Feb 01, 2022 at 08:08:23PM +0000, Yosry Ahmed wrote:
> > Currently memcg stats show several types of kernel memory:
> > kernel stack, page tables, sock, vmalloc, and slab.
> > However, there are other allocations with __GFP_ACCOUNT
> > (or supersets such as GFP_KERNEL_ACCOUNT) that are not accounted
> > in any of those stats, a few examples are:
> > - various kvm allocations (e.g. allocated pages to create vcpus)
> > - io_uring
> > - tmp_page in pipes during pipe_write()
> > - bpf ringbuffers
> > - unix sockets
> >
> > Keeping track of the total kernel memory is essential for the ease of
> > migration from cgroup v1 to v2 as there are large discrepancies between
> > v1's kmem.usage_in_bytes and the sum of the available kernel memory
stats
> > in v2. Adding separate memcg stats for all __GFP_ACCOUNT kernel
> > allocations is an impractical maintenance burden as there a lot of those
> > all over the kernel code, with more use cases likely to show up in the
> > future.
>
> No objection, I'm just curious how it makes migration to v2 easier in
> particular. Or is it just that you've used the v1 stat to track
> application regressions and would like to continue doing that in v2?

We are using "memory.kmem.usage_in_bytes" in v1 to get the total kernel
memory accounted to a memcg to maintain historical data of jobs memory
(anon, file, kernel), and for debugging purposes. In v2 there is no
equivalent.

We found that the total of other existing v2 kernel memory stats (vmalloc,
slab, stack, ..) is very different for some workloads compared to v1's
"memory.kmem.usage_in_bytes". We need a v2 indicator of the total kernel
memory accounted to a memcg."

>
> > Therefore, add a "kernel" memcg stat that is analogous to kmem
> > page counter, with added benefits such as using rstat infrastructure
> > which aggregates stats more efficiently. Additionally, this provides a
> > lighter alternative in case the legacy kmem is deprecated in the future
> >
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > ---
> >  Documentation/admin-guide/cgroup-v2.rst |  5 +++++
> >  include/linux/memcontrol.h              |  1 +
> >  mm/memcontrol.c                         | 24 ++++++++++++++++++------
> >  3 files changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst
b/Documentation/admin-guide/cgroup-v2.rst
> > index 5aa368d165da..a0027d570a7f 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > @@ -1317,6 +1317,11 @@ PAGE_SIZE multiple when read back.
> >         vmalloc (npn)
> >               Amount of memory used for vmap backed memory.
> >
> > +       kernel (npn)
> > +             Amount of total kernel memory, including
> > +             (kernel_stack, pagetables, percpu, vmalloc, slab) in
> > +             addition to other kernel memory use cases.
> > +
> >         shmem
> >               Amount of cached filesystem data that is swap-backed,
> >               such as tmpfs, shm segments, shared anonymous mmap()s
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index b72d75141e12..fa51986365a4 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -34,6 +34,7 @@ enum memcg_stat_item {
> >       MEMCG_SOCK,
> >       MEMCG_PERCPU_B,
> >       MEMCG_VMALLOC,
> > +     MEMCG_KMEM,
> >       MEMCG_NR_STAT,
> >  };
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 09d342c7cbd0..c55d7056ac98 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1376,6 +1376,7 @@ static const struct memory_stat memory_stats[] = {
> >       { "percpu",                     MEMCG_PERCPU_B                  },
> >       { "sock",                       MEMCG_SOCK                      },
> >       { "vmalloc",                    MEMCG_VMALLOC                   },
> > +     { "kernel",                     MEMCG_KMEM                      },
>
> It's a superset of percpu, sock, vmalloc etc., so please move it ahead
> of them.
>
>         anon
>         file
>         kernel
>         kernel_stack
>         pagetables
>         ...
>
> and in the doc as well.
>

Done in v2.

> >       { "shmem",                      NR_SHMEM                        },
> >       { "file_mapped",                NR_FILE_MAPPED                  },
> >       { "file_dirty",                 NR_FILE_DIRTY                   },
> > @@ -2979,6 +2980,19 @@ static void memcg_free_cache_id(int id)
> >       ida_simple_remove(&memcg_cache_ida, id);
> >  }
> >
> > +static void mem_cgroup_kmem_record(struct mem_cgroup *memcg,
> > +                                int nr_pages)
>
> No real need for the namespace prefix since it's a static
> function. How about account_kmem()? Avoids the line wrap, too.

Does memcg_account_kmem() sound good? It avoids the line wrap too and is
more consistent with most static functions in the file that have either a
"memcg_" or "mem_cgroup_" prefix.

Thanks!

[-- Attachment #2: Type: text/html, Size: 6590 bytes --]

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 21:00 ` Johannes Weiner
  2022-02-01 22:01   ` Yosry Ahmed
@ 2022-02-01 22:10   ` Yosry Ahmed
  1 sibling, 0 replies; 8+ messages in thread
From: Yosry Ahmed @ 2022-02-01 22:10 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Michal Hocko, Muchun Song, Shakeel Butt,
	linux-kernel, linux-mm

Hello Johannes,

Thanks so much for your review.

On Tue, Feb 1, 2022 at 1:00 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> Hello Yosry,
>
> On Tue, Feb 01, 2022 at 08:08:23PM +0000, Yosry Ahmed wrote:
> > Currently memcg stats show several types of kernel memory:
> > kernel stack, page tables, sock, vmalloc, and slab.
> > However, there are other allocations with __GFP_ACCOUNT
> > (or supersets such as GFP_KERNEL_ACCOUNT) that are not accounted
> > in any of those stats, a few examples are:
> > - various kvm allocations (e.g. allocated pages to create vcpus)
> > - io_uring
> > - tmp_page in pipes during pipe_write()
> > - bpf ringbuffers
> > - unix sockets
> >
> > Keeping track of the total kernel memory is essential for the ease of
> > migration from cgroup v1 to v2 as there are large discrepancies between
> > v1's kmem.usage_in_bytes and the sum of the available kernel memory stats
> > in v2. Adding separate memcg stats for all __GFP_ACCOUNT kernel
> > allocations is an impractical maintenance burden as there a lot of those
> > all over the kernel code, with more use cases likely to show up in the
> > future.
>
> No objection, I'm just curious how it makes migration to v2 easier in
> particular. Or is it just that you've used the v1 stat to track
> application regressions and would like to continue doing that in v2?
>

We are using "memory.kmem.usage_in_bytes" in v1 to get the total
kernel memory accounted to a memcg to maintain historical data of jobs
memory (anon, file, kernel), and for debugging purposes. In v2 there
is no equivalent.

We found that the total of other existing v2 kernel memory stats
(vmalloc, slab, stack, ..) is very different for some workloads
compared to v1's "memory.kmem.usage_in_bytes". We need a v2 indicator
of the total kernel memory accounted to a memcg."

> > Therefore, add a "kernel" memcg stat that is analogous to kmem
> > page counter, with added benefits such as using rstat infrastructure
> > which aggregates stats more efficiently. Additionally, this provides a
> > lighter alternative in case the legacy kmem is deprecated in the future
> >
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > ---
> >  Documentation/admin-guide/cgroup-v2.rst |  5 +++++
> >  include/linux/memcontrol.h              |  1 +
> >  mm/memcontrol.c                         | 24 ++++++++++++++++++------
> >  3 files changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > index 5aa368d165da..a0027d570a7f 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > @@ -1317,6 +1317,11 @@ PAGE_SIZE multiple when read back.
> >         vmalloc (npn)
> >               Amount of memory used for vmap backed memory.
> >
> > +       kernel (npn)
> > +             Amount of total kernel memory, including
> > +             (kernel_stack, pagetables, percpu, vmalloc, slab) in
> > +             addition to other kernel memory use cases.
> > +
> >         shmem
> >               Amount of cached filesystem data that is swap-backed,
> >               such as tmpfs, shm segments, shared anonymous mmap()s
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index b72d75141e12..fa51986365a4 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -34,6 +34,7 @@ enum memcg_stat_item {
> >       MEMCG_SOCK,
> >       MEMCG_PERCPU_B,
> >       MEMCG_VMALLOC,
> > +     MEMCG_KMEM,
> >       MEMCG_NR_STAT,
> >  };
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 09d342c7cbd0..c55d7056ac98 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1376,6 +1376,7 @@ static const struct memory_stat memory_stats[] = {
> >       { "percpu",                     MEMCG_PERCPU_B                  },
> >       { "sock",                       MEMCG_SOCK                      },
> >       { "vmalloc",                    MEMCG_VMALLOC                   },
> > +     { "kernel",                     MEMCG_KMEM                      },
>
> It's a superset of percpu, sock, vmalloc etc., so please move it ahead
> of them.
>
>         anon
>         file
>         kernel
>         kernel_stack
>         pagetables
>         ...
>
> and in the doc as well.
>

Done in v2.

> >       { "shmem",                      NR_SHMEM                        },
> >       { "file_mapped",                NR_FILE_MAPPED                  },
> >       { "file_dirty",                 NR_FILE_DIRTY                   },
> > @@ -2979,6 +2980,19 @@ static void memcg_free_cache_id(int id)
> >       ida_simple_remove(&memcg_cache_ida, id);
> >  }
> >
> > +static void mem_cgroup_kmem_record(struct mem_cgroup *memcg,
> > +                                int nr_pages)
>
> No real need for the namespace prefix since it's a static
> function. How about account_kmem()? Avoids the line wrap, too.

Does memcg_account_kmem() sound good? It avoids the line wrap too and
is more consistent with most static functions in the file that have
either a "memcg_" or "mem_cgroup_" prefix.

Thanks!

>
> Otherwise, looks good to me, so with those changes:
>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Thanks!

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 20:08 [PATCH] memcg: add per-memcg total kernel memory stat Yosry Ahmed
  2022-02-01 20:26 ` Shakeel Butt
  2022-02-01 21:00 ` Johannes Weiner
@ 2022-02-02  0:47 ` kernel test robot
  2022-02-02  5:41   ` kernel test robot
  3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-02  0:47 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]

Hi Yosry,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.17-rc2 next-20220201]
[cannot apply to tj-cgroup/for-next hnaz-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: x86_64-randconfig-a006-20220131 (https://download.01.org/0day-ci/archive/20220202/202202020822.IjosSkm5-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/c956773fbdc99f19a2f19427595473fd591ccdc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
        git checkout c956773fbdc99f19a2f19427595473fd591ccdc3
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/memcontrol.c: In function 'uncharge_batch':
>> mm/memcontrol.c:6817:4: error: implicit declaration of function 'mem_cgroup_kmem_record'; did you mean 'mem_cgroup_id_remove'? [-Werror=implicit-function-declaration]
    6817 |    mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
         |    ^~~~~~~~~~~~~~~~~~~~~~
         |    mem_cgroup_id_remove
   cc1: some warnings being treated as errors


vim +6817 mm/memcontrol.c

  6807	
  6808	static void uncharge_batch(const struct uncharge_gather *ug)
  6809	{
  6810		unsigned long flags;
  6811	
  6812		if (ug->nr_memory) {
  6813			page_counter_uncharge(&ug->memcg->memory, ug->nr_memory);
  6814			if (do_memsw_account())
  6815				page_counter_uncharge(&ug->memcg->memsw, ug->nr_memory);
  6816			if (ug->nr_kmem)
> 6817				mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
  6818			memcg_oom_recover(ug->memcg);
  6819		}
  6820	
  6821		local_irq_save(flags);
  6822		__count_memcg_events(ug->memcg, PGPGOUT, ug->pgpgout);
  6823		__this_cpu_add(ug->memcg->vmstats_percpu->nr_page_events, ug->nr_memory);
  6824		memcg_check_events(ug->memcg, ug->nid);
  6825		local_irq_restore(flags);
  6826	
  6827		/* drop reference from uncharge_folio */
  6828		css_put(&ug->memcg->css);
  6829	}
  6830	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
  2022-02-01 20:08 [PATCH] memcg: add per-memcg total kernel memory stat Yosry Ahmed
@ 2022-02-02  5:41   ` kernel test robot
  2022-02-01 21:00 ` Johannes Weiner
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-02  5:41 UTC (permalink / raw)
  To: Yosry Ahmed; +Cc: llvm, kbuild-all

Hi Yosry,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.17-rc2]
[cannot apply to tj-cgroup/for-next hnaz-mm/master next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: hexagon-randconfig-r045-20220130 (https://download.01.org/0day-ci/archive/20220202/202202021308.Lv9zjwOg-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c956773fbdc99f19a2f19427595473fd591ccdc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
        git checkout c956773fbdc99f19a2f19427595473fd591ccdc3
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> mm/memcontrol.c:6817:4: error: implicit declaration of function 'mem_cgroup_kmem_record' [-Werror,-Wimplicit-function-declaration]
                           mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
                           ^
   1 error generated.


vim +/mem_cgroup_kmem_record +6817 mm/memcontrol.c

  6807	
  6808	static void uncharge_batch(const struct uncharge_gather *ug)
  6809	{
  6810		unsigned long flags;
  6811	
  6812		if (ug->nr_memory) {
  6813			page_counter_uncharge(&ug->memcg->memory, ug->nr_memory);
  6814			if (do_memsw_account())
  6815				page_counter_uncharge(&ug->memcg->memsw, ug->nr_memory);
  6816			if (ug->nr_kmem)
> 6817				mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
  6818			memcg_oom_recover(ug->memcg);
  6819		}
  6820	
  6821		local_irq_save(flags);
  6822		__count_memcg_events(ug->memcg, PGPGOUT, ug->pgpgout);
  6823		__this_cpu_add(ug->memcg->vmstats_percpu->nr_page_events, ug->nr_memory);
  6824		memcg_check_events(ug->memcg, ug->nid);
  6825		local_irq_restore(flags);
  6826	
  6827		/* drop reference from uncharge_folio */
  6828		css_put(&ug->memcg->css);
  6829	}
  6830	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH] memcg: add per-memcg total kernel memory stat
@ 2022-02-02  5:41   ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-02  5:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3060 bytes --]

Hi Yosry,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.17-rc2]
[cannot apply to tj-cgroup/for-next hnaz-mm/master next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: hexagon-randconfig-r045-20220130 (https://download.01.org/0day-ci/archive/20220202/202202021308.Lv9zjwOg-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c956773fbdc99f19a2f19427595473fd591ccdc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yosry-Ahmed/memcg-add-per-memcg-total-kernel-memory-stat/20220202-041007
        git checkout c956773fbdc99f19a2f19427595473fd591ccdc3
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> mm/memcontrol.c:6817:4: error: implicit declaration of function 'mem_cgroup_kmem_record' [-Werror,-Wimplicit-function-declaration]
                           mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
                           ^
   1 error generated.


vim +/mem_cgroup_kmem_record +6817 mm/memcontrol.c

  6807	
  6808	static void uncharge_batch(const struct uncharge_gather *ug)
  6809	{
  6810		unsigned long flags;
  6811	
  6812		if (ug->nr_memory) {
  6813			page_counter_uncharge(&ug->memcg->memory, ug->nr_memory);
  6814			if (do_memsw_account())
  6815				page_counter_uncharge(&ug->memcg->memsw, ug->nr_memory);
  6816			if (ug->nr_kmem)
> 6817				mem_cgroup_kmem_record(ug->memcg, -ug->nr_kmem);
  6818			memcg_oom_recover(ug->memcg);
  6819		}
  6820	
  6821		local_irq_save(flags);
  6822		__count_memcg_events(ug->memcg, PGPGOUT, ug->pgpgout);
  6823		__this_cpu_add(ug->memcg->vmstats_percpu->nr_page_events, ug->nr_memory);
  6824		memcg_check_events(ug->memcg, ug->nid);
  6825		local_irq_restore(flags);
  6826	
  6827		/* drop reference from uncharge_folio */
  6828		css_put(&ug->memcg->css);
  6829	}
  6830	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

end of thread, other threads:[~2022-02-02  5:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 20:08 [PATCH] memcg: add per-memcg total kernel memory stat Yosry Ahmed
2022-02-01 20:26 ` Shakeel Butt
2022-02-01 21:00 ` Johannes Weiner
2022-02-01 22:01   ` Yosry Ahmed
2022-02-01 22:10   ` Yosry Ahmed
2022-02-02  0:47 ` kernel test robot
2022-02-02  5:41 ` kernel test robot
2022-02-02  5:41   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.