memcg: drop kmem.limit_in_bytes

PATCH In review Michal Hocko 5 participants 6 messages Jul 4, 2023 – Jul 7, 2023

Subsystems (4)

Memory management Documentation cgroup memcg

lore

Share & tools

lore

Michal Hocko

author maintainer

2023-07-04 11:52 UTC #0 lore raw

PATCH 1/1 memcg: drop kmem.limit_in_bytes

From: Michal Hocko <[email protected]>

kmem.limit_in_bytes (v1 way to limit kernel memory usage) has been deprecated since 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes") merged in 5.16. We haven't heard about any serious users since then but it seems that the mere presence of the file is causing more harm thatn good. We (SUSE) have had several bug reports from customers where Docker based containers started to fail because a write to kmem.limit_in_bytes has failed.

This was unexpected because runc code only expects ENOENT (kmem disabled) or EBUSY (tasks already running within cgroup). So a new error code was unexpected and the whole container startup failed. This has been later addressed by
https://github.com/opencontainers/runc/commit/52390d68040637dfc77f9fda6bbe70952423d380
so current Docker runtimes do not suffer from the problem anymore. There are still older version of Docker in use and likely hard to get rid of completely.

Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.

I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").

Cc stable

Signed-off-by Michal Hocko <[email protected]>

---
 Documentation/admin-guide/cgroup-v1/memory.rst |  2 --
 mm/memcontrol.c                                | 13 -------------
 2 files changed, 15 deletions(-)

Documentation/admin-guide/cgroup-v1/memory.rst +0 -2

index 47d1d7d932a8..b92c71f39172 100644
@@ -92,8 +92,6 @@ Brief summary of control files.
  memory.oom_control		     set/show oom controls.
  memory.numa_stat		     show the number of memory usage per numa
 				     node
- memory.kmem.limit_in_bytes          This knob is deprecated and writing to
-                                     it will return -ENOTSUPP.
  memory.kmem.usage_in_bytes          show current kernel memory allocation
  memory.kmem.failcnt                 show the number of kernel memory usage
 				     hits limits

mm/memcontrol.c +0 ~~-13~~

index 4b27e245a055..a0d3ed8d02e2 100644
@@ -3750,9 +3750,6 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
 	case _MEMSWAP:
 		counter = &memcg->memsw;
 		break;
-	case _KMEM:
-		counter = &memcg->kmem;
-		break;
 	case _TCP:
 		counter = &memcg->tcpmem;
 		break;

@@ -3913,10 +3910,6 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		case _MEMSWAP:
 			ret = mem_cgroup_resize_max(memcg, nr_pages, true);
 			break;
-		case _KMEM:
-			/* kmem.limit_in_bytes is deprecated. */
-			ret = -EOPNOTSUPP;
-			break;
 		case _TCP:
 			ret = memcg_update_tcp_max(memcg, nr_pages);
 			break;

@@ -5132,12 +5125,6 @@ static struct cftype mem_cgroup_legacy_files[] = {
 		.seq_show = memcg_numa_stat_show,
 	},
 #endif
-	{
-		.name = "kmem.limit_in_bytes",
-		.private = MEMFILE_PRIVATE(_KMEM, RES_LIMIT),
-		.write = mem_cgroup_write,
-		.read_u64 = mem_cgroup_read_u64,
-	},
 	{
 		.name = "kmem.usage_in_bytes",
 		.private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),

Signature · 1 line

2.30.2

Shakeel Butt

Acked-by on v1 1/1

2023-07-05 05:13 UTC #1 lore raw

Quoted 19 lines · Michal Hocko

From: Michal Hocko <[email protected]>
kmem.limit_in_bytes (v1 way to limit kernel memory usage) has been deprecated since 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes") merged in 5.16. We haven't heard about any serious users since then but it seems that the mere presence of the file is causing more harm thatn good. We (SUSE) have had several bug reports from customers where Docker based containers started to fail because a write to kmem.limit_in_bytes has failed.
This was unexpected because runc code only expects ENOENT (kmem disabled) or EBUSY (tasks already running within cgroup). So a new error code was unexpected and the whole container startup failed. This has been later addressed by
https://github.com/opencontainers/runc/commit/52390d68040637dfc77f9fda6bbe70952423d380
so current Docker runtimes do not suffer from the problem anymore. There are still older version of Docker in use and likely hard to get rid of completely.

Quoted 8 lines

Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.
I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").
Cc stable
Signed-off-by Michal Hocko <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Johannes Weiner

maintainer

2023-07-05 13:44 UTC #2 lore raw

In reply to Michal Hocko

Quoted 50 lines · Michal Hocko

From: Michal Hocko <[email protected]>
kmem.limit_in_bytes (v1 way to limit kernel memory usage) has been deprecated since 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes") merged in 5.16. We haven't heard about any serious users since then but it seems that the mere presence of the file is causing more harm thatn good. We (SUSE) have had several bug reports from customers where Docker based containers started to fail because a write to kmem.limit_in_bytes has failed.
This was unexpected because runc code only expects ENOENT (kmem disabled) or EBUSY (tasks already running within cgroup). So a new error code was unexpected and the whole container startup failed. This has been later addressed by
https://github.com/opencontainers/runc/commit/52390d68040637dfc77f9fda6bbe70952423d380
so current Docker runtimes do not suffer from the problem anymore. There are still older version of Docker in use and likely hard to get rid of completely.
Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.
I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").
Cc stable
Signed-off-by Michal Hocko <[email protected]>
---
 Documentation/admin-guide/cgroup-v1/memory.rst |  2 --
 mm/memcontrol.c                                | 13 -------------
 2 files changed, 15 deletions(-)
Documentation/admin-guide/cgroup-v1/memory.rst +0 -2
index 47d1d7d932a8..b92c71f39172 100644
@@ -92,8 +92,6 @@ Brief summary of control files.
  memory.oom_control		     set/show oom controls.
  memory.numa_stat		     show the number of memory usage per numa
 				     node
- memory.kmem.limit_in_bytes          This knob is deprecated and writing to
-                                     it will return -ENOTSUPP.
  memory.kmem.usage_in_bytes          show current kernel memory allocation
  memory.kmem.failcnt                 show the number of kernel memory usage
 				     hits limits
mm/memcontrol.c +0 -0
index 4b27e245a055..a0d3ed8d02e2 100644

mm/memcontrol.c static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css, · line 3755

Quoted 9 lines

mm/memcontrol.c +0 -3

@@ -3750,9 +3750,6 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
 	case _MEMSWAP:
 		counter = &memcg->memsw;
 		break;
-	case _KMEM:
-		counter = &memcg->kmem;
-		break;
 	case _TCP:
 		counter = &memcg->tcpmem;
 		break;

This case is still needed for the remaining kmem files:

	{
		.name = "kmem.usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.failcnt",
		.private = MEMFILE_PRIVATE(_KMEM, RES_FAILCNT),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.max_usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_MAX_USAGE),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},

otherwise they BUG() when reading.

Without this hunk, the patch looks good to me.

Michal Hocko

2023-07-07 07:07 UTC #3 lore raw

On Wed 05-07-23 09:44:34, Johannes Weiner wrote:
[...]

Quoted 35 lines

Quoted 5 lines

mm/memcontrol.c +0 -0
index 4b27e245a055..a0d3ed8d02e2 100644

mm/memcontrol.c static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css, · line 3755

Quoted 9 lines

mm/memcontrol.c +0 -3

@@ -3750,9 +3750,6 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
 	case _MEMSWAP:
 		counter = &memcg->memsw;
 		break;
-	case _KMEM:
-		counter = &memcg->kmem;
-		break;
 	case _TCP:
 		counter = &memcg->tcpmem;
 		break;

This case is still needed for the remaining kmem files:

	{
		.name = "kmem.usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.failcnt",
		.private = MEMFILE_PRIVATE(_KMEM, RES_FAILCNT),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.max_usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_MAX_USAGE),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},

Quoted 3 lines

otherwise they BUG() when reading.
Without this hunk, the patch looks good to me.

You are right! Thanks. The updated patch below. Andrew could you replace by this version please?
---
From b1bbc63b6baff9c5aaf30393cec29112b3abca25 Mon Sep 17 00:00:00 2001
From: Michal Hocko <[email protected]>
Date: Tue, 4 Jul 2023 13:38:11 +0200
Subject: [PATCH] memcg: drop kmem.limit_in_bytes

Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.

I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").

Cc stable

Signed-off-by Michal Hocko <[email protected]>

---
 Documentation/admin-guide/cgroup-v1/memory.rst |  2 --
 mm/memcontrol.c                                | 10 ----------
 2 files changed, 12 deletions(-)

Documentation/admin-guide/cgroup-v1/memory.rst +0 -2

index fabaad3fd9c2..8d3afeede10e 100644
@@ -92,8 +92,6 @@ Brief summary of control files.
  memory.oom_control		     set/show oom controls.
  memory.numa_stat		     show the number of memory usage per numa
 				     node
- memory.kmem.limit_in_bytes          This knob is deprecated and writing to
-                                     it will return -ENOTSUPP.
  memory.kmem.usage_in_bytes          show current kernel memory allocation
  memory.kmem.failcnt                 show the number of kernel memory usage
 				     hits limits

mm/memcontrol.c +0 ~~-10~~

index e8ca4bdcb03c..ab99503c9ff2 100644
@@ -3871,10 +3871,6 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		case _MEMSWAP:
 			ret = mem_cgroup_resize_max(memcg, nr_pages, true);
 			break;
-		case _KMEM:
-			/* kmem.limit_in_bytes is deprecated. */
-			ret = -EOPNOTSUPP;
-			break;
 		case _TCP:
 			ret = memcg_update_tcp_max(memcg, nr_pages);
 			break;

@@ -5085,12 +5081,6 @@ static struct cftype mem_cgroup_legacy_files[] = {
 		.seq_show = memcg_numa_stat_show,
 	},
 #endif
-	{
-		.name = "kmem.limit_in_bytes",
-		.private = MEMFILE_PRIVATE(_KMEM, RES_LIMIT),
-		.write = mem_cgroup_write,
-		.read_u64 = mem_cgroup_read_u64,
-	},
 	{
 		.name = "kmem.usage_in_bytes",
 		.private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),

Signature · 7 lines

2.30.2



-- 
Michal Hocko
SUSE Labs

Johannes Weiner

maintainer Acked-by

2023-07-07 13:50 UTC #4 lore raw

Quoted 22 lines · Michal Hocko

From b1bbc63b6baff9c5aaf30393cec29112b3abca25 Mon Sep 17 00:00:00 2001
From: Michal Hocko <[email protected]>
Date: Tue, 4 Jul 2023 13:38:11 +0200
Subject: [PATCH] memcg: drop kmem.limit_in_bytes
kmem.limit_in_bytes (v1 way to limit kernel memory usage) has been deprecated since 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes") merged in 5.16. We haven't heard about any serious users since then but it seems that the mere presence of the file is causing more harm thatn good. We (SUSE) have had several bug reports from customers where Docker based containers started to fail because a write to kmem.limit_in_bytes has failed.
This was unexpected because runc code only expects ENOENT (kmem disabled) or EBUSY (tasks already running within cgroup). So a new error code was unexpected and the whole container startup failed. This has been later addressed by
https://github.com/opencontainers/runc/commit/52390d68040637dfc77f9fda6bbe70952423d380
so current Docker runtimes do not suffer from the problem anymore. There are still older version of Docker in use and likely hard to get rid of completely.

Quoted 8 lines

Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.
I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").
Cc stable
Signed-off-by Michal Hocko <[email protected]>

Acked-by Johannes Weiner <[email protected]>

Roman Gushchin

maintainer Acked-by

2023-07-07 16:40 UTC #5 lore raw

In reply to Michal Hocko

Quoted 66 lines · Michal Hocko

On Wed 05-07-23 09:44:34, Johannes Weiner wrote:
[...]
Quoted 35 lines
Quoted 5 lines
mm/memcontrol.c +0 -0
index 4b27e245a055..a0d3ed8d02e2 100644
mm/memcontrol.c static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css, · line 3755
Quoted 9 lines
mm/memcontrol.c +0 -3
@@ -3750,9 +3750,6 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
 	case _MEMSWAP:
 		counter = &memcg->memsw;
 		break;
-	case _KMEM:
-		counter = &memcg->kmem;
-		break;
 	case _TCP:
 		counter = &memcg->tcpmem;
 		break;
This case is still needed for the remaining kmem files:
	{
		.name = "kmem.usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.failcnt",
		.private = MEMFILE_PRIVATE(_KMEM, RES_FAILCNT),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},
	{
		.name = "kmem.max_usage_in_bytes",
		.private = MEMFILE_PRIVATE(_KMEM, RES_MAX_USAGE),
		.write = mem_cgroup_reset,
		.read_u64 = mem_cgroup_read_u64,
	},
Quoted 3 lines
otherwise they BUG() when reading.
Without this hunk, the patch looks good to me.
You are right! Thanks. The updated patch below. Andrew could you replace by this version please?
---
From b1bbc63b6baff9c5aaf30393cec29112b3abca25 Mon Sep 17 00:00:00 2001
From: Michal Hocko <[email protected]>
Date: Tue, 4 Jul 2023 13:38:11 +0200
Subject: [PATCH] memcg: drop kmem.limit_in_bytes
kmem.limit_in_bytes (v1 way to limit kernel memory usage) has been deprecated since 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes") merged in 5.16. We haven't heard about any serious users since then but it seems that the mere presence of the file is causing more harm thatn good. We (SUSE) have had several bug reports from customers where Docker based containers started to fail because a write to kmem.limit_in_bytes has failed.
This was unexpected because runc code only expects ENOENT (kmem disabled) or EBUSY (tasks already running within cgroup). So a new error code was unexpected and the whole container startup failed. This has been later addressed by
https://github.com/opencontainers/runc/commit/52390d68040637dfc77f9fda6bbe70952423d380
so current Docker runtimes do not suffer from the problem anymore. There are still older version of Docker in use and likely hard to get rid of completely.

Quoted 8 lines

Address this by wiping out the file completely and effectively get back to pre 4.5 era and CONFIG_MEMCG_KMEM=n configuration.
I would recommend backporting to stable trees which have picked up 58056f77502f ("memcg, kmem: further deprecate kmem.limit_in_bytes").
Cc stable
Signed-off-by Michal Hocko <[email protected]>

Acked-by Roman Gushchin <[email protected]>

Thanks!