Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1286741 > unrolled thread

[PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

Started byJohannes Weiner <hannes@cmpxchg.org>
First post2015-12-08 19:40 +0100
Last post2015-12-10 15:30 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller Johannes Weiner <hannes@cmpxchg.org> - 2015-12-08 19:40 +0100
    Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Vladimir Davydov <vdavydov@virtuozzo.com> - 2015-12-09 12:40 +0100
      Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Johannes Weiner <hannes@cmpxchg.org> - 2015-12-09 15:40 +0100
      Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Michal Hocko <mhocko@kernel.org> - 2015-12-10 14:30 +0100
        Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Johannes Weiner <hannes@cmpxchg.org> - 2015-12-10 16:20 +0100
          Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Michal Hocko <mhocko@kernel.org> - 2015-12-10 17:30 +0100
    Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2  memory controller Michal Hocko <mhocko@kernel.org> - 2015-12-10 15:30 +0100

#1286741 — [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromJohannes Weiner <hannes@cmpxchg.org>
Date2015-12-08 19:40 +0100
Subject[PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qDvl8-5U7-13@gated-at.bofh.it>
The original cgroup memory controller has an extension to account slab
memory (and other "kernel memory" consumers) in a separate "kmem"
counter, once the user set an explicit limit on that "kmem" pool.

However, this includes various consumers whose sizes are directly
linked to userspace activity. Accounting them as an optional "kmem"
extension is problematic for several reasons:

1. It leaves the main memory interface with incomplete semantics. A
   user who puts their workload into a cgroup and configures a memory
   limit does not expect us to leave holes in the containment as big
   as the dentry and inode cache, or the kernel stack pages.

2. If the limit set on this random historical subgroup of consumers is
   reached, subsequent allocations will fail even when the main memory
   pool available to the cgroup is not yet exhausted and/or has
   reclaimable memory in it.

3. Calling it 'kernel memory' is misleading. The dentry and inode
   caches are no more 'kernel' (or no less 'user') memory than the
   page cache itself. Treating these consumers as different classes is
   a historical implementation detail that should not leak to users.

So, in addition to page cache, anonymous memory, and network socket
memory, account the following memory consumers per default in the
cgroup2 memory controller:

     - threadinfo
     - task_struct
     - task_delay_info
     - pid
     - cred
     - mm_struct
     - vm_area_struct and vm_region (nommu)
     - anon_vma and anon_vma_chain
     - signal_struct
     - sighand_struct
     - fs_struct
     - files_struct
     - fdtable and fdtable->full_fds_bits
     - dentry and external_name
     - inode for all filesystems.

This should give us reasonable memory isolation for most common
workloads out of the box.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/memcontrol.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ab72c47..d048137 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2356,13 +2356,14 @@ int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
 	if (!memcg_kmem_online(memcg))
 		return 0;
 
-	if (!page_counter_try_charge(&memcg->kmem, nr_pages, &counter))
-		return -ENOMEM;
-
 	ret = try_charge(memcg, gfp, nr_pages);
-	if (ret) {
-		page_counter_uncharge(&memcg->kmem, nr_pages);
+	if (ret)
 		return ret;
+
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) &&
+	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) {
+		cancel_charge(memcg, nr_pages);
+		return -ENOMEM;
 	}
 
 	page->mem_cgroup = memcg;
@@ -2391,7 +2392,9 @@ void __memcg_kmem_uncharge(struct page *page, int order)
 
 	VM_BUG_ON_PAGE(mem_cgroup_is_root(memcg), page);
 
-	page_counter_uncharge(&memcg->kmem, nr_pages);
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
+		page_counter_uncharge(&memcg->kmem, nr_pages);
+
 	page_counter_uncharge(&memcg->memory, nr_pages);
 	if (do_memsw_account())
 		page_counter_uncharge(&memcg->memsw, nr_pages);
@@ -2895,7 +2898,8 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
 	 * onlined after this point, because it has at least one child
 	 * already.
 	 */
-	if (memcg_kmem_online(parent))
+	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) ||
+	    memcg_kmem_online(parent))
 		ret = memcg_online_kmem(memcg);
 	mutex_unlock(&memcg_limit_mutex);
 	return ret;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1287434 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromVladimir Davydov <vdavydov@virtuozzo.com>
Date2015-12-09 12:40 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qDLge-7LW-9@gated-at.bofh.it>
In reply to#1286741
On Tue, Dec 08, 2015 at 01:34:24PM -0500, Johannes Weiner wrote:
> The original cgroup memory controller has an extension to account slab
> memory (and other "kernel memory" consumers) in a separate "kmem"
> counter, once the user set an explicit limit on that "kmem" pool.
> 
> However, this includes various consumers whose sizes are directly
> linked to userspace activity. Accounting them as an optional "kmem"
> extension is problematic for several reasons:
> 
> 1. It leaves the main memory interface with incomplete semantics. A
>    user who puts their workload into a cgroup and configures a memory
>    limit does not expect us to leave holes in the containment as big
>    as the dentry and inode cache, or the kernel stack pages.
> 
> 2. If the limit set on this random historical subgroup of consumers is
>    reached, subsequent allocations will fail even when the main memory
>    pool available to the cgroup is not yet exhausted and/or has
>    reclaimable memory in it.
> 
> 3. Calling it 'kernel memory' is misleading. The dentry and inode
>    caches are no more 'kernel' (or no less 'user') memory than the
>    page cache itself. Treating these consumers as different classes is
>    a historical implementation detail that should not leak to users.
> 
> So, in addition to page cache, anonymous memory, and network socket
> memory, account the following memory consumers per default in the
> cgroup2 memory controller:
> 
>      - threadinfo
>      - task_struct
>      - task_delay_info
>      - pid
>      - cred
>      - mm_struct
>      - vm_area_struct and vm_region (nommu)
>      - anon_vma and anon_vma_chain
>      - signal_struct
>      - sighand_struct
>      - fs_struct
>      - files_struct
>      - fdtable and fdtable->full_fds_bits
>      - dentry and external_name
>      - inode for all filesystems.
> 
> This should give us reasonable memory isolation for most common
> workloads out of the box.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Vladimir Davydov <vdavydov@virtuozzo.com>

The patch looks good to me, but I think we still need to add a boot-time
knob to disable kmem accounting, as we do for sockets:

From: Vladimir Davydov <vdavydov@virtuozzo.com>
Subject: [PATCH] mm: memcontrol: allow to disable kmem accounting for cgroup2

Kmem accounting might incur overhead that some users can't put up with.
Besides, the implementation is still considered unstable. So let's
provide a way to disable it for those users who aren't happy with it.

To disable kmem accounting for cgroup2, pass cgroup.memory=nokmem at
boot time.

Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c1bda3bbb7db..1b7a85dc6013 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -602,6 +602,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	cgroup.memory=	[KNL] Pass options to the cgroup memory controller.
 			Format: <string>
 			nosocket -- Disable socket memory accounting.
+			nokmem -- Disable kernel memory accounting.
 
 	checkreqprot	[SELINUX] Set initial checkreqprot flag value.
 			Format: { "0" | "1" }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6faea81e66d7..6a5572241dc6 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -83,6 +83,9 @@ struct mem_cgroup *root_mem_cgroup __read_mostly;
 /* Socket memory accounting disabled? */
 static bool cgroup_memory_nosocket;
 
+/* Kernel memory accounting disabled? */
+static bool cgroup_memory_nokmem;
+
 /* Whether the swap controller is active */
 #ifdef CONFIG_MEMCG_SWAP
 int do_swap_account __read_mostly;
@@ -2898,8 +2901,8 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
 	 * onlined after this point, because it has at least one child
 	 * already.
 	 */
-	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) ||
-	    memcg_kmem_online(parent))
+	if (memcg_kmem_online(parent) ||
+	    (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nokmem))
 		ret = memcg_online_kmem(memcg);
 	mutex_unlock(&memcg_limit_mutex);
 	return ret;
@@ -5587,6 +5590,8 @@ static int __init cgroup_memory(char *s)
 			continue;
 		if (!strcmp(token, "nosocket"))
 			cgroup_memory_nosocket = true;
+		if (!strcmp(token, "nokmem"))
+			cgroup_memory_nokmem = true;
 	}
 	return 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1287525 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromJohannes Weiner <hannes@cmpxchg.org>
Date2015-12-09 15:40 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qDO4p-1aj-1@gated-at.bofh.it>
In reply to#1287434
On Wed, Dec 09, 2015 at 02:30:38PM +0300, Vladimir Davydov wrote:
> On Tue, Dec 08, 2015 at 01:34:24PM -0500, Johannes Weiner wrote:
> > The original cgroup memory controller has an extension to account slab
> > memory (and other "kernel memory" consumers) in a separate "kmem"
> > counter, once the user set an explicit limit on that "kmem" pool.
> > 
> > However, this includes various consumers whose sizes are directly
> > linked to userspace activity. Accounting them as an optional "kmem"
> > extension is problematic for several reasons:
> > 
> > 1. It leaves the main memory interface with incomplete semantics. A
> >    user who puts their workload into a cgroup and configures a memory
> >    limit does not expect us to leave holes in the containment as big
> >    as the dentry and inode cache, or the kernel stack pages.
> > 
> > 2. If the limit set on this random historical subgroup of consumers is
> >    reached, subsequent allocations will fail even when the main memory
> >    pool available to the cgroup is not yet exhausted and/or has
> >    reclaimable memory in it.
> > 
> > 3. Calling it 'kernel memory' is misleading. The dentry and inode
> >    caches are no more 'kernel' (or no less 'user') memory than the
> >    page cache itself. Treating these consumers as different classes is
> >    a historical implementation detail that should not leak to users.
> > 
> > So, in addition to page cache, anonymous memory, and network socket
> > memory, account the following memory consumers per default in the
> > cgroup2 memory controller:
> > 
> >      - threadinfo
> >      - task_struct
> >      - task_delay_info
> >      - pid
> >      - cred
> >      - mm_struct
> >      - vm_area_struct and vm_region (nommu)
> >      - anon_vma and anon_vma_chain
> >      - signal_struct
> >      - sighand_struct
> >      - fs_struct
> >      - files_struct
> >      - fdtable and fdtable->full_fds_bits
> >      - dentry and external_name
> >      - inode for all filesystems.
> > 
> > This should give us reasonable memory isolation for most common
> > workloads out of the box.
> > 
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> Acked-by: Vladimir Davydov <vdavydov@virtuozzo.com>

Thank you!

> The patch looks good to me, but I think we still need to add a boot-time
> knob to disable kmem accounting, as we do for sockets:
> 
> From: Vladimir Davydov <vdavydov@virtuozzo.com>
> Subject: [PATCH] mm: memcontrol: allow to disable kmem accounting for cgroup2
> 
> Kmem accounting might incur overhead that some users can't put up with.
> Besides, the implementation is still considered unstable. So let's
> provide a way to disable it for those users who aren't happy with it.
> 
> To disable kmem accounting for cgroup2, pass cgroup.memory=nokmem at
> boot time.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

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

Especially in the early release phases, there might be birthing pain
that users in the field would want to work around. And I'd rather they
can selectively disable problematic parts during the transition than
switching back wholesale to the old cgroup interface.

For me that would be the prime reason: a temporary workaround for
legacy users until we get our stuff sorted out. Unacceptable overhead
or instability would be something we would have to address anyway.
And then it's fine too that the flag continues to use the historic
misnomer "kmem".
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288512 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromMichal Hocko <mhocko@kernel.org>
Date2015-12-10 14:30 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qE9se-6M6-13@gated-at.bofh.it>
In reply to#1287434
On Wed 09-12-15 14:30:38, Vladimir Davydov wrote:
> From: Vladimir Davydov <vdavydov@virtuozzo.com>
> Subject: [PATCH] mm: memcontrol: allow to disable kmem accounting for cgroup2
> 
> Kmem accounting might incur overhead that some users can't put up with.
> Besides, the implementation is still considered unstable. So let's
> provide a way to disable it for those users who aren't happy with it.

Yes there will be users who do not want to pay an additional overhead
and still accoplish what they need.
I haven't measured the overhead lately - especially after the opt-out ->
opt-in change so it might be much lower than my previous ~5% for kbuild
load.
 
> To disable kmem accounting for cgroup2, pass cgroup.memory=nokmem at
> boot time.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index c1bda3bbb7db..1b7a85dc6013 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -602,6 +602,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  	cgroup.memory=	[KNL] Pass options to the cgroup memory controller.
>  			Format: <string>
>  			nosocket -- Disable socket memory accounting.
> +			nokmem -- Disable kernel memory accounting.
>  
>  	checkreqprot	[SELINUX] Set initial checkreqprot flag value.
>  			Format: { "0" | "1" }
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6faea81e66d7..6a5572241dc6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -83,6 +83,9 @@ struct mem_cgroup *root_mem_cgroup __read_mostly;
>  /* Socket memory accounting disabled? */
>  static bool cgroup_memory_nosocket;
>  
> +/* Kernel memory accounting disabled? */
> +static bool cgroup_memory_nokmem;
> +
>  /* Whether the swap controller is active */
>  #ifdef CONFIG_MEMCG_SWAP
>  int do_swap_account __read_mostly;
> @@ -2898,8 +2901,8 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
>  	 * onlined after this point, because it has at least one child
>  	 * already.
>  	 */
> -	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) ||
> -	    memcg_kmem_online(parent))
> +	if (memcg_kmem_online(parent) ||
> +	    (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nokmem))
>  		ret = memcg_online_kmem(memcg);
>  	mutex_unlock(&memcg_limit_mutex);
>  	return ret;
> @@ -5587,6 +5590,8 @@ static int __init cgroup_memory(char *s)
>  			continue;
>  		if (!strcmp(token, "nosocket"))
>  			cgroup_memory_nosocket = true;
> +		if (!strcmp(token, "nokmem"))
> +			cgroup_memory_nokmem = true;
>  	}
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288573 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromJohannes Weiner <hannes@cmpxchg.org>
Date2015-12-10 16:20 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qEbaF-7V7-15@gated-at.bofh.it>
In reply to#1288512
On Thu, Dec 10, 2015 at 02:28:33PM +0100, Michal Hocko wrote:
> On Wed 09-12-15 14:30:38, Vladimir Davydov wrote:
> > From: Vladimir Davydov <vdavydov@virtuozzo.com>
> > Subject: [PATCH] mm: memcontrol: allow to disable kmem accounting for cgroup2
> > 
> > Kmem accounting might incur overhead that some users can't put up with.
> > Besides, the implementation is still considered unstable. So let's
> > provide a way to disable it for those users who aren't happy with it.
> 
> Yes there will be users who do not want to pay an additional overhead
> and still accoplish what they need.
> I haven't measured the overhead lately - especially after the opt-out ->
> opt-in change so it might be much lower than my previous ~5% for kbuild
> load.

I think switching from accounting *all* slab allocations to accounting
a list of, what, less than 20 select slabs, counts as a change
significant enough to entirely invalidate those measurements and never
bring up that number again in the context of kmem cost, don't you think?

There isn't that much that the kmem is doing, but for posterity I ran
kbuild test inside a cgroup2, with and without cgroup.memory=nokmem,
and these are the results:

default:
 Performance counter stats for 'make -j16 -s clean bzImage' (3 runs):

     715823.047005      task-clock (msec)         #    3.794 CPUs utilized          
           252,538      context-switches          #    0.353 K/sec                  
            32,018      cpu-migrations            #    0.045 K/sec                  
        16,678,202      page-faults               #    0.023 M/sec                  
 1,783,804,914,980      cycles                    #    2.492 GHz                    
   <not supported>      stalled-cycles-frontend  
   <not supported>      stalled-cycles-backend   
 1,346,424,021,728      instructions              #    0.75  insns per cycle        
   298,744,956,474      branches                  #  417.363 M/sec                  
    10,207,872,737      branch-misses             #    3.42% of all branches        

     188.667608149 seconds time elapsed                                          ( +-  0.66% )

cgroup.memory=nokmem
 Performance counter stats for 'make -j16 -s clean bzImage' (3 runs):

     729028.322760      task-clock (msec)         #    3.805 CPUs utilized          
           258,775      context-switches          #    0.356 K/sec                  
            32,241      cpu-migrations            #    0.044 K/sec                  
        16,647,817      page-faults               #    0.023 M/sec                  
 1,816,827,061,194      cycles                    #    2.497 GHz                    
   <not supported>      stalled-cycles-frontend  
   <not supported>      stalled-cycles-backend   
 1,345,446,962,095      instructions              #    0.74  insns per cycle        
   298,461,034,326      branches                  #  410.277 M/sec                  
    10,215,145,963      branch-misses             #    3.42% of all branches        

     191.583957742 seconds time elapsed                                          ( +-  0.57% )

I would say the difference is solidly in the noise.

I also profiled a silly find | xargs stat pipe to excercise the dentry
and inode accounting, and this was the highest kmem-specific entry:

     0.27%     0.27%  find             [kernel.kallsyms]        [k] __memcg_kmem_get_cache                    
                       |
                       ---__memcg_kmem_get_cache
                          __kmalloc
                          ext4_htree_store_dirent
                          htree_dirblock_to_tree
                          ext4_htree_fill_tree
                          ext4_readdir
                          iterate_dir
                          sys_getdents
                          entry_SYSCALL_64_fastpath
                          __getdents64

So can we *please* lay this whole "unreasonable burden to legacy and
power users" line of argument to rest and get on with our work? And
then tackle scalability problems as they show up in real workloads?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288650 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromMichal Hocko <mhocko@kernel.org>
Date2015-12-10 17:30 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qEcgr-8O-27@gated-at.bofh.it>
In reply to#1288573
On Thu 10-12-15 10:16:27, Johannes Weiner wrote:
> On Thu, Dec 10, 2015 at 02:28:33PM +0100, Michal Hocko wrote:
> > On Wed 09-12-15 14:30:38, Vladimir Davydov wrote:
> > > From: Vladimir Davydov <vdavydov@virtuozzo.com>
> > > Subject: [PATCH] mm: memcontrol: allow to disable kmem accounting for cgroup2
> > > 
> > > Kmem accounting might incur overhead that some users can't put up with.
> > > Besides, the implementation is still considered unstable. So let's
> > > provide a way to disable it for those users who aren't happy with it.
> > 
> > Yes there will be users who do not want to pay an additional overhead
> > and still accoplish what they need.
> > I haven't measured the overhead lately - especially after the opt-out ->
> > opt-in change so it might be much lower than my previous ~5% for kbuild
> > load.
> 
> I think switching from accounting *all* slab allocations to accounting
> a list of, what, less than 20 select slabs, counts as a change
> significant enough to entirely invalidate those measurements and never
> bring up that number again in the context of kmem cost, don't you think?

Yes, as I've said the numbers are expected to be much lower. That is
one of the reasons I have acknowledged kmem enabled as a reasonable
default.  There will always be _special_ loads where numbers might look
differently, though, and having a disabling knob is a reasonable thing
to offer with a minimum maintenance overhead. And this is the argument
for the inclusion of the patch from Vladimir.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288534 — Re: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller

FromMichal Hocko <mhocko@kernel.org>
Date2015-12-10 15:30 +0100
SubjectRe: [PATCH 7/8] mm: memcontrol: account "kmem" consumers in cgroup2 memory controller
Message-ID<qEaoh-7nL-5@gated-at.bofh.it>
In reply to#1286741
On Tue 08-12-15 13:34:24, Johannes Weiner wrote:
> The original cgroup memory controller has an extension to account slab
> memory (and other "kernel memory" consumers) in a separate "kmem"
> counter, once the user set an explicit limit on that "kmem" pool.
> 
> However, this includes various consumers whose sizes are directly
> linked to userspace activity. Accounting them as an optional "kmem"
> extension is problematic for several reasons:
> 
> 1. It leaves the main memory interface with incomplete semantics. A
>    user who puts their workload into a cgroup and configures a memory
>    limit does not expect us to leave holes in the containment as big
>    as the dentry and inode cache, or the kernel stack pages.
> 
> 2. If the limit set on this random historical subgroup of consumers is
>    reached, subsequent allocations will fail even when the main memory
>    pool available to the cgroup is not yet exhausted and/or has
>    reclaimable memory in it.
> 
> 3. Calling it 'kernel memory' is misleading. The dentry and inode
>    caches are no more 'kernel' (or no less 'user') memory than the
>    page cache itself. Treating these consumers as different classes is
>    a historical implementation detail that should not leak to users.
> 
> So, in addition to page cache, anonymous memory, and network socket
> memory, account the following memory consumers per default in the
> cgroup2 memory controller:
> 
>      - threadinfo
>      - task_struct
>      - task_delay_info
>      - pid
>      - cred
>      - mm_struct
>      - vm_area_struct and vm_region (nommu)
>      - anon_vma and anon_vma_chain
>      - signal_struct
>      - sighand_struct
>      - fs_struct
>      - files_struct
>      - fdtable and fdtable->full_fds_bits
>      - dentry and external_name
>      - inode for all filesystems.
> 
> This should give us reasonable memory isolation for most common
> workloads out of the box.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memcontrol.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ab72c47..d048137 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2356,13 +2356,14 @@ int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
>  	if (!memcg_kmem_online(memcg))
>  		return 0;
>  
> -	if (!page_counter_try_charge(&memcg->kmem, nr_pages, &counter))
> -		return -ENOMEM;
> -
>  	ret = try_charge(memcg, gfp, nr_pages);
> -	if (ret) {
> -		page_counter_uncharge(&memcg->kmem, nr_pages);
> +	if (ret)
>  		return ret;
> +
> +	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) &&
> +	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) {
> +		cancel_charge(memcg, nr_pages);
> +		return -ENOMEM;
>  	}
>  
>  	page->mem_cgroup = memcg;
> @@ -2391,7 +2392,9 @@ void __memcg_kmem_uncharge(struct page *page, int order)
>  
>  	VM_BUG_ON_PAGE(mem_cgroup_is_root(memcg), page);
>  
> -	page_counter_uncharge(&memcg->kmem, nr_pages);
> +	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> +		page_counter_uncharge(&memcg->kmem, nr_pages);
> +
>  	page_counter_uncharge(&memcg->memory, nr_pages);
>  	if (do_memsw_account())
>  		page_counter_uncharge(&memcg->memsw, nr_pages);
> @@ -2895,7 +2898,8 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
>  	 * onlined after this point, because it has at least one child
>  	 * already.
>  	 */
> -	if (memcg_kmem_online(parent))
> +	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) ||
> +	    memcg_kmem_online(parent))
>  		ret = memcg_online_kmem(memcg);
>  	mutex_unlock(&memcg_limit_mutex);
>  	return ret;
> -- 
> 2.6.3

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web