linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1
@ 2024-06-28 21:03 Roman Gushchin
  2024-06-28 21:03 ` [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c Roman Gushchin
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

This patchset puts all cgroup v1's members of struct mem_cgroup,
struct mem_cgroup_per_node and struct task_struct under
the CONFIG_MEMCG_V1 config option. If cgroup v1 support is not
required (and it's true for many cgroup users these days), it
allows to save a bit of memory and compile out some code, some
of which is on relatively hot paths. It also structures the code
a bit better by grouping cgroup v1-specific stuff in one place.


Roman Gushchin (9):
  mm: memcg: move memcg_account_kmem() to memcontrol-v1.c
  mm: memcg: factor out legacy socket memory accounting code
  mm: memcg: guard cgroup v1-specific code in
    mem_cgroup_print_oom_meminfo()
  mm: memcg: gather memcg1-specific fields initialization in
    memcg1_memcg_init()
  mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c
  mm: memcg: put memcg1-specific struct mem_cgroup's members under
    CONFIG_MEMCG_V1
  mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node
  mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1
  mm: memcg: put struct task_struct::in_user_fault under CONFIG_MEMCG_V1

 include/linux/memcontrol.h | 147 +++++++++++++++++++------------------
 include/linux/sched.h      |   6 +-
 mm/memcontrol-v1.c         |  38 ++++++++++
 mm/memcontrol-v1.h         |  20 +++++
 mm/memcontrol.c            |  70 +++++++-----------
 5 files changed, 164 insertions(+), 117 deletions(-)

-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:30   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code Roman Gushchin
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

memcg_account_kmem() consists of a trivial statistics change via
mod_memcg_state() call and a relatively large memcg1-specific part.

Let's factor out the mod_memcg_state() call and move the rest into
the mm/memcontrol-v1.c file. Also rename memcg_account_kmem()
into memcg1_account_kmem() for consistency.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 mm/memcontrol-v1.c | 12 ++++++++++++
 mm/memcontrol-v1.h |  2 ++
 mm/memcontrol.c    | 31 ++++++++++---------------------
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 6ac47954eefc..c73a0ff0cc39 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -2913,6 +2913,18 @@ struct cftype memsw_files[] = {
 	{ },	/* terminate */
 };
 
+#ifdef CONFIG_MEMCG_KMEM
+void memcg1_account_kmem(struct mem_cgroup *memcg, int 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);
+	}
+}
+#endif /* CONFIG_MEMCG_KMEM */
+
 static int __init memcg1_init(void)
 {
 	int node;
diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index 64b053d7f131..61620e2b0bf0 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -107,6 +107,7 @@ void memcg1_check_events(struct mem_cgroup *memcg, int nid);
 
 void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s);
 
+void memcg1_account_kmem(struct mem_cgroup *memcg, int nr_pages);
 extern struct cftype memsw_files[];
 extern struct cftype mem_cgroup_legacy_files[];
 
@@ -125,6 +126,7 @@ static inline void memcg1_check_events(struct mem_cgroup *memcg, int nid) {}
 
 static inline void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) {}
 
+static inline void memcg1_account_kmem(struct mem_cgroup *memcg, int nr_pages) {}
 extern struct cftype memsw_files[];
 extern struct cftype mem_cgroup_legacy_files[];
 #endif	/* CONFIG_MEMCG_V1 */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c251bbe35f4b..802a077e2e2f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1718,7 +1718,6 @@ static DEFINE_MUTEX(percpu_charge_mutex);
 static struct obj_cgroup *drain_obj_stock(struct memcg_stock_pcp *stock);
 static bool obj_stock_flush_required(struct memcg_stock_pcp *stock,
 				     struct mem_cgroup *root_memcg);
-static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages);
 
 #else
 static inline struct obj_cgroup *drain_obj_stock(struct memcg_stock_pcp *stock)
@@ -1730,9 +1729,6 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock,
 {
 	return false;
 }
-static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages)
-{
-}
 #endif
 
 /**
@@ -2642,18 +2638,6 @@ struct obj_cgroup *get_obj_cgroup_from_folio(struct folio *folio)
 	return objcg;
 }
 
-static void memcg_account_kmem(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
@@ -2666,7 +2650,8 @@ static void obj_cgroup_uncharge_pages(struct obj_cgroup *objcg,
 
 	memcg = get_mem_cgroup_from_objcg(objcg);
 
-	memcg_account_kmem(memcg, -nr_pages);
+	mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
+	memcg1_account_kmem(memcg, -nr_pages);
 	refill_stock(memcg, nr_pages);
 
 	css_put(&memcg->css);
@@ -2692,7 +2677,8 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp,
 	if (ret)
 		goto out;
 
-	memcg_account_kmem(memcg, nr_pages);
+	mod_memcg_state(memcg, MEMCG_KMEM, nr_pages);
+	memcg1_account_kmem(memcg, nr_pages);
 out:
 	css_put(&memcg->css);
 
@@ -2845,7 +2831,8 @@ static struct obj_cgroup *drain_obj_stock(struct memcg_stock_pcp *stock)
 
 			memcg = get_mem_cgroup_from_objcg(old);
 
-			memcg_account_kmem(memcg, -nr_pages);
+			mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
+			memcg1_account_kmem(memcg, -nr_pages);
 			__refill_stock(memcg, nr_pages);
 
 			css_put(&memcg->css);
@@ -4806,8 +4793,10 @@ 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 (ug->nr_kmem)
-			memcg_account_kmem(ug->memcg, -ug->nr_kmem);
+		if (ug->nr_kmem) {
+			mod_memcg_state(ug->memcg, MEMCG_KMEM, -ug->nr_kmem);
+			memcg1_account_kmem(ug->memcg, -ug->nr_kmem);
+		}
 		memcg1_oom_recover(ug->memcg);
 	}
 
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
  2024-06-28 21:03 ` [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:39   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo() Roman Gushchin
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

Move out the legacy cgroup v1 socket memory accounting code into
mm/memcontrol-v1.c.

This commit introduces three new functions: memcg1_tcpmem_active(),
memcg1_charge_skmem() and memcg1_uncharge_skmem(), which contain
all cgroup v1-specific code and become trivial if CONFIG_MEMCG_V1
isn't set.

Note, that !!memcg->tcpmem_pressure check in
mem_cgroup_under_socket_pressure() can't be easily moved into
memcontrol-v1.h without including memcontrol-v1.h from memcontrol.h
which isn't a good idea, so it's better to just #ifdef it.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 include/linux/memcontrol.h |  2 ++
 mm/memcontrol-v1.c         | 17 +++++++++++++++++
 mm/memcontrol-v1.h         | 16 ++++++++++++++++
 mm/memcontrol.c            | 22 +++++-----------------
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 796cfa842346..44ab6394c9ed 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1650,8 +1650,10 @@ void mem_cgroup_sk_alloc(struct sock *sk);
 void mem_cgroup_sk_free(struct sock *sk);
 static inline bool mem_cgroup_under_socket_pressure(struct mem_cgroup *memcg)
 {
+#ifdef CONFIG_MEMCG_V1
 	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
 		return !!memcg->tcpmem_pressure;
+#endif /* CONFIG_MEMCG_V1 */
 	do {
 		if (time_before(jiffies, READ_ONCE(memcg->socket_pressure)))
 			return true;
diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index c73a0ff0cc39..c9b4c3e4797d 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -2925,6 +2925,23 @@ void memcg1_account_kmem(struct mem_cgroup *memcg, int nr_pages)
 }
 #endif /* CONFIG_MEMCG_KMEM */
 
+bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
+			 gfp_t gfp_mask)
+{
+	struct page_counter *fail;
+
+	if (page_counter_try_charge(&memcg->tcpmem, nr_pages, &fail)) {
+		memcg->tcpmem_pressure = 0;
+		return true;
+	}
+	memcg->tcpmem_pressure = 1;
+	if (gfp_mask & __GFP_NOFAIL) {
+		page_counter_charge(&memcg->tcpmem, nr_pages);
+		return true;
+	}
+	return false;
+}
+
 static int __init memcg1_init(void)
 {
 	int node;
diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index 61620e2b0bf0..c8e5119223bb 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -108,6 +108,17 @@ void memcg1_check_events(struct mem_cgroup *memcg, int nid);
 void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s);
 
 void memcg1_account_kmem(struct mem_cgroup *memcg, int nr_pages);
+static inline bool memcg1_tcpmem_active(struct mem_cgroup *memcg)
+{
+	return memcg->tcpmem_active;
+}
+bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
+			 gfp_t gfp_mask);
+static inline void memcg1_uncharge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages)
+{
+	page_counter_uncharge(&memcg->tcpmem, nr_pages);
+}
+
 extern struct cftype memsw_files[];
 extern struct cftype mem_cgroup_legacy_files[];
 
@@ -127,6 +138,11 @@ static inline void memcg1_check_events(struct mem_cgroup *memcg, int nid) {}
 static inline void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) {}
 
 static inline void memcg1_account_kmem(struct mem_cgroup *memcg, int nr_pages) {}
+static inline bool memcg1_tcpmem_active(struct mem_cgroup *memcg) { return false; }
+static inline bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
+				       gfp_t gfp_mask) { return true; }
+static inline void memcg1_uncharge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages) {}
+
 extern struct cftype memsw_files[];
 extern struct cftype mem_cgroup_legacy_files[];
 #endif	/* CONFIG_MEMCG_V1 */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 802a077e2e2f..2c0605bbbb31 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3788,7 +3788,7 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
 	if (cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_nosocket)
 		static_branch_dec(&memcg_sockets_enabled_key);
 
-	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg->tcpmem_active)
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg1_tcpmem_active(memcg))
 		static_branch_dec(&memcg_sockets_enabled_key);
 
 #if defined(CONFIG_MEMCG_KMEM)
@@ -5013,7 +5013,7 @@ void mem_cgroup_sk_alloc(struct sock *sk)
 	memcg = mem_cgroup_from_task(current);
 	if (mem_cgroup_is_root(memcg))
 		goto out;
-	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && !memcg->tcpmem_active)
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && !memcg1_tcpmem_active(memcg))
 		goto out;
 	if (css_tryget(&memcg->css))
 		sk->sk_memcg = memcg;
@@ -5039,20 +5039,8 @@ void mem_cgroup_sk_free(struct sock *sk)
 bool mem_cgroup_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
 			     gfp_t gfp_mask)
 {
-	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
-		struct page_counter *fail;
-
-		if (page_counter_try_charge(&memcg->tcpmem, nr_pages, &fail)) {
-			memcg->tcpmem_pressure = 0;
-			return true;
-		}
-		memcg->tcpmem_pressure = 1;
-		if (gfp_mask & __GFP_NOFAIL) {
-			page_counter_charge(&memcg->tcpmem, nr_pages);
-			return true;
-		}
-		return false;
-	}
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
+		return memcg1_charge_skmem(memcg, nr_pages, gfp_mask);
 
 	if (try_charge(memcg, gfp_mask, nr_pages) == 0) {
 		mod_memcg_state(memcg, MEMCG_SOCK, nr_pages);
@@ -5070,7 +5058,7 @@ bool mem_cgroup_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
 void mem_cgroup_uncharge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages)
 {
 	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
-		page_counter_uncharge(&memcg->tcpmem, nr_pages);
+		memcg1_uncharge_skmem(memcg, nr_pages);
 		return;
 	}
 
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo()
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
  2024-06-28 21:03 ` [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c Roman Gushchin
  2024-06-28 21:03 ` [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:40   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init() Roman Gushchin
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

Put cgroup v1-specific code in mem_cgroup_print_oom_meminfo() under
CONFIG_MEMCG_V1.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 mm/memcontrol.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2c0605bbbb31..b69abd327549 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1532,6 +1532,7 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
 		pr_info("swap: usage %llukB, limit %llukB, failcnt %lu\n",
 			K((u64)page_counter_read(&memcg->swap)),
 			K((u64)READ_ONCE(memcg->swap.max)), memcg->swap.failcnt);
+#ifdef CONFIG_MEMCG_V1
 	else {
 		pr_info("memory+swap: usage %llukB, limit %llukB, failcnt %lu\n",
 			K((u64)page_counter_read(&memcg->memsw)),
@@ -1540,6 +1541,7 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
 			K((u64)page_counter_read(&memcg->kmem)),
 			K((u64)memcg->kmem.max), memcg->kmem.failcnt);
 	}
+#endif
 
 	pr_info("Memory cgroup stats for ");
 	pr_cont_cgroup_path(memcg->css.cgroup);
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init()
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (2 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo() Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:43   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c Roman Gushchin
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

Gather all memcg1-specific struct mem_cgroup's members initialization
in a new memcg1_memcg_init() function, defined in mm/memcontrol-v1.c.
Obviously, if CONFIG_MEMCG_V1 is not set, there is no need to
initialize these fields, so the function becomes trivial.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 mm/memcontrol-v1.c | 9 +++++++++
 mm/memcontrol-v1.h | 2 ++
 mm/memcontrol.c    | 6 +-----
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index c9b4c3e4797d..5fa48a45e34b 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -1961,6 +1961,15 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
 	return ret;
 }
 
+void memcg1_memcg_init(struct mem_cgroup *memcg)
+{
+	INIT_LIST_HEAD(&memcg->oom_notify);
+	mutex_init(&memcg->thresholds_lock);
+	spin_lock_init(&memcg->move_lock);
+	INIT_LIST_HEAD(&memcg->event_list);
+	spin_lock_init(&memcg->event_list_lock);
+}
+
 void memcg1_css_offline(struct mem_cgroup *memcg)
 {
 	struct mem_cgroup_event *event, *tmp;
diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index c8e5119223bb..6545fa1b7d09 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -76,6 +76,7 @@ int memory_stat_show(struct seq_file *m, void *v);
 
 /* Cgroup v1-specific declarations */
 #ifdef CONFIG_MEMCG_V1
+void memcg1_memcg_init(struct mem_cgroup *memcg);
 void memcg1_remove_from_trees(struct mem_cgroup *memcg);
 
 static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg)
@@ -124,6 +125,7 @@ extern struct cftype mem_cgroup_legacy_files[];
 
 #else	/* CONFIG_MEMCG_V1 */
 
+static inline void memcg1_memcg_init(struct mem_cgroup *memcg) {}
 static inline void memcg1_remove_from_trees(struct mem_cgroup *memcg) {}
 static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg) {}
 static inline bool memcg1_wait_acct_move(struct mem_cgroup *memcg) { return false; }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b69abd327549..e78ed54d46d2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3624,13 +3624,9 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 		goto fail;
 
 	INIT_WORK(&memcg->high_work, high_work_func);
-	INIT_LIST_HEAD(&memcg->oom_notify);
-	mutex_init(&memcg->thresholds_lock);
-	spin_lock_init(&memcg->move_lock);
 	vmpressure_init(&memcg->vmpressure);
-	INIT_LIST_HEAD(&memcg->event_list);
-	spin_lock_init(&memcg->event_list_lock);
 	memcg->socket_pressure = jiffies;
+	memcg1_memcg_init(memcg);
 #ifdef CONFIG_MEMCG_KMEM
 	memcg->kmemcg_id = -1;
 	INIT_LIST_HEAD(&memcg->objcg_list);
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (3 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init() Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:49   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

There are only few memcg1-specific struct mem_cgroup's members
accesses left in mm/memcontrol.c. Let's guard them with the
CONFIG_MEMCG_V1 config option.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 mm/memcontrol.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e78ed54d46d2..661e3a70e685 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3672,20 +3672,23 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 	page_counter_set_high(&memcg->swap, PAGE_COUNTER_MAX);
 	if (parent) {
 		WRITE_ONCE(memcg->swappiness, mem_cgroup_swappiness(parent));
-		WRITE_ONCE(memcg->oom_kill_disable, READ_ONCE(parent->oom_kill_disable));
 
 		page_counter_init(&memcg->memory, &parent->memory);
 		page_counter_init(&memcg->swap, &parent->swap);
+#ifdef CONFIG_MEMCG_V1
+		WRITE_ONCE(memcg->oom_kill_disable, READ_ONCE(parent->oom_kill_disable));
 		page_counter_init(&memcg->kmem, &parent->kmem);
 		page_counter_init(&memcg->tcpmem, &parent->tcpmem);
+#endif
 	} else {
 		init_memcg_stats();
 		init_memcg_events();
 		page_counter_init(&memcg->memory, NULL);
 		page_counter_init(&memcg->swap, NULL);
+#ifdef CONFIG_MEMCG_V1
 		page_counter_init(&memcg->kmem, NULL);
 		page_counter_init(&memcg->tcpmem, NULL);
-
+#endif
 		root_mem_cgroup = memcg;
 		return &memcg->css;
 	}
@@ -3820,8 +3823,10 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css)
 
 	page_counter_set_max(&memcg->memory, PAGE_COUNTER_MAX);
 	page_counter_set_max(&memcg->swap, PAGE_COUNTER_MAX);
+#ifdef CONFIG_MEMCG_V1
 	page_counter_set_max(&memcg->kmem, PAGE_COUNTER_MAX);
 	page_counter_set_max(&memcg->tcpmem, PAGE_COUNTER_MAX);
+#endif
 	page_counter_set_min(&memcg->memory, 0);
 	page_counter_set_low(&memcg->memory, 0);
 	page_counter_set_high(&memcg->memory, PAGE_COUNTER_MAX);
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (4 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:48   ` Shakeel Butt
  2024-07-03 15:12   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node Roman Gushchin
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

Put memcg1-specific members of struct mem_cgroup under the
CONFIG_MEMCG_V1 config option. Also group them close to the end
of struct mem_cgroup just before the dynamic per-node part.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 include/linux/memcontrol.h | 103 +++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 50 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 44ab6394c9ed..107b0c5d6eab 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -188,10 +188,6 @@ struct mem_cgroup {
 		struct page_counter memsw;	/* v1 only */
 	};
 
-	/* Legacy consumer-oriented counters */
-	struct page_counter kmem;		/* v1 only */
-	struct page_counter tcpmem;		/* v1 only */
-
 	/* Range enforcement for interrupt charges */
 	struct work_struct high_work;
 
@@ -205,8 +201,6 @@ struct mem_cgroup {
 	bool zswap_writeback;
 #endif
 
-	unsigned long soft_limit;
-
 	/* vmpressure notifications */
 	struct vmpressure vmpressure;
 
@@ -215,13 +209,7 @@ struct mem_cgroup {
 	 */
 	bool oom_group;
 
-	/* protected by memcg_oom_lock */
-	bool		oom_lock;
-	int		under_oom;
-
-	int	swappiness;
-	/* OOM-Killer disable */
-	int		oom_kill_disable;
+	int swappiness;
 
 	/* memory.events and memory.events.local */
 	struct cgroup_file events_file;
@@ -230,27 +218,6 @@ struct mem_cgroup {
 	/* handle for "memory.swap.events" */
 	struct cgroup_file swap_events_file;
 
-	/* protect arrays of thresholds */
-	struct mutex thresholds_lock;
-
-	/* thresholds for memory usage. RCU-protected */
-	struct mem_cgroup_thresholds thresholds;
-
-	/* thresholds for mem+swap usage. RCU-protected */
-	struct mem_cgroup_thresholds memsw_thresholds;
-
-	/* For oom notifier event fd */
-	struct list_head oom_notify;
-
-	/*
-	 * Should we move charges of a task when a task is moved into this
-	 * mem_cgroup ? And what type of charges should we move ?
-	 */
-	unsigned long move_charge_at_immigrate;
-	/* taken only while moving_account > 0 */
-	spinlock_t		move_lock;
-	unsigned long		move_lock_flags;
-
 	CACHELINE_PADDING(_pad1_);
 
 	/* memory.stat */
@@ -267,10 +234,6 @@ struct mem_cgroup {
 	 */
 	unsigned long		socket_pressure;
 
-	/* Legacy tcp memory accounting */
-	bool			tcpmem_active;
-	int			tcpmem_pressure;
-
 #ifdef CONFIG_MEMCG_KMEM
 	int kmemcg_id;
 	/*
@@ -284,14 +247,6 @@ struct mem_cgroup {
 	struct list_head objcg_list;
 #endif
 
-	CACHELINE_PADDING(_pad2_);
-
-	/*
-	 * set > 0 if pages under this cgroup are moving to other cgroup.
-	 */
-	atomic_t		moving_account;
-	struct task_struct	*move_lock_task;
-
 	struct memcg_vmstats_percpu __percpu *vmstats_percpu;
 
 #ifdef CONFIG_CGROUP_WRITEBACK
@@ -300,10 +255,6 @@ struct mem_cgroup {
 	struct memcg_cgwb_frn cgwb_frn[MEMCG_CGWB_FRN_CNT];
 #endif
 
-	/* List of events which userspace want to receive */
-	struct list_head event_list;
-	spinlock_t event_list_lock;
-
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	struct deferred_split deferred_split_queue;
 #endif
@@ -313,6 +264,58 @@ struct mem_cgroup {
 	struct lru_gen_mm_list mm_list;
 #endif
 
+#ifdef CONFIG_MEMCG_V1
+	/* Legacy consumer-oriented counters */
+	struct page_counter kmem;		/* v1 only */
+	struct page_counter tcpmem;		/* v1 only */
+
+	unsigned long soft_limit;
+
+	/* protected by memcg_oom_lock */
+	bool oom_lock;
+	int under_oom;
+
+	/* OOM-Killer disable */
+	int oom_kill_disable;
+
+	/* protect arrays of thresholds */
+	struct mutex thresholds_lock;
+
+	/* thresholds for memory usage. RCU-protected */
+	struct mem_cgroup_thresholds thresholds;
+
+	/* thresholds for mem+swap usage. RCU-protected */
+	struct mem_cgroup_thresholds memsw_thresholds;
+
+	/* For oom notifier event fd */
+	struct list_head oom_notify;
+
+	/*
+	 * Should we move charges of a task when a task is moved into this
+	 * mem_cgroup ? And what type of charges should we move ?
+	 */
+	unsigned long move_charge_at_immigrate;
+	/* taken only while moving_account > 0 */
+	spinlock_t move_lock;
+	unsigned long move_lock_flags;
+
+	/* Legacy tcp memory accounting */
+	bool tcpmem_active;
+	int tcpmem_pressure;
+
+	CACHELINE_PADDING(_pad2_);
+
+	/*
+	 * set > 0 if pages under this cgroup are moving to other cgroup.
+	 */
+	atomic_t moving_account;
+	struct task_struct *move_lock_task;
+
+	/* List of events which userspace want to receive */
+	struct list_head event_list;
+	spinlock_t event_list_lock;
+#endif /* CONFIG_MEMCG_V1 */
+
 	struct mem_cgroup_per_node *nodeinfo[];
 };
 
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (5 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1 Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:52   ` Shakeel Butt
  2024-07-03 15:13   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1 Roman Gushchin
  2024-06-28 21:03 ` [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault " Roman Gushchin
  8 siblings, 2 replies; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

Put memcg1-specific members of struct mem_cgroup_per_node under the
CONFIG_MEMCG_V1 config option.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 include/linux/memcontrol.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 107b0c5d6eab..c7ef628ee882 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -92,6 +92,7 @@ struct mem_cgroup_per_node {
 	struct lruvec_stats			*lruvec_stats;
 	struct shrinker_info __rcu	*shrinker_info;
 
+#ifdef CONFIG_MEMCG_V1
 	/*
 	 * Memcg-v1 only stuff in middle as buffer between read mostly fields
 	 * and update often fields to avoid false sharing. Once v1 stuff is
@@ -102,6 +103,7 @@ struct mem_cgroup_per_node {
 	unsigned long		usage_in_excess;/* Set to the value by which */
 						/* the soft limit is exceeded*/
 	bool			on_tree;
+#endif
 
 	/* Fields which get updated often at the end. */
 	struct lruvec		lruvec;
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (6 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:53   ` Shakeel Butt
  2024-06-28 21:03 ` [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault " Roman Gushchin
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

The memcg_in_oom field of the struct task_struct is not used by
the cgroup v2's memory controller, so it can be happily compiled out
if CONFIG_MEMCG_V1 is not set.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 include/linux/sched.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 61591ac6eab6..2a16023e8620 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1447,9 +1447,11 @@ struct task_struct {
 	unsigned int			kcov_softirq;
 #endif
 
-#ifdef CONFIG_MEMCG
+#ifdef CONFIG_MEMCG_V1
 	struct mem_cgroup		*memcg_in_oom;
+#endif
 
+#ifdef CONFIG_MEMCG
 	/* Number of pages to reclaim on returning to userland: */
 	unsigned int			memcg_nr_pages_over_high;
 
-- 
2.45.2.803.g4e1b14247a-goog



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

* [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault under CONFIG_MEMCG_V1
  2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
                   ` (7 preceding siblings ...)
  2024-06-28 21:03 ` [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1 Roman Gushchin
@ 2024-06-28 21:03 ` Roman Gushchin
  2024-06-29  0:55   ` Shakeel Butt
  8 siblings, 1 reply; 23+ messages in thread
From: Roman Gushchin @ 2024-06-28 21:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Johannes Weiner, Michal Hocko,
	Shakeel Butt, Muchun Song, Roman Gushchin

The struct task_struct's in_user_fault member is not used by the
cgroup v2's memory controller, so it can be put under the
CONFIG_MEMCG_V1 config option. To do so, mem_cgroup_enter_user_fault()
and mem_cgroup_exit_user_fault() are moved under the CONFIG_MEMCG_V1
option as well.

Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 include/linux/memcontrol.h | 40 +++++++++++++++++++-------------------
 include/linux/sched.h      |  2 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index c7ef628ee882..d0c9365ff039 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -943,18 +943,6 @@ void mem_cgroup_print_oom_context(struct mem_cgroup *memcg,
 
 void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg);
 
-static inline void mem_cgroup_enter_user_fault(void)
-{
-	WARN_ON(current->in_user_fault);
-	current->in_user_fault = 1;
-}
-
-static inline void mem_cgroup_exit_user_fault(void)
-{
-	WARN_ON(!current->in_user_fault);
-	current->in_user_fault = 0;
-}
-
 struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
 					    struct mem_cgroup *oom_domain);
 void mem_cgroup_print_oom_group(struct mem_cgroup *memcg);
@@ -1402,14 +1390,6 @@ static inline void mem_cgroup_handle_over_high(gfp_t gfp_mask)
 {
 }
 
-static inline void mem_cgroup_enter_user_fault(void)
-{
-}
-
-static inline void mem_cgroup_exit_user_fault(void)
-{
-}
-
 static inline struct mem_cgroup *mem_cgroup_get_oom_group(
 	struct task_struct *victim, struct mem_cgroup *oom_domain)
 {
@@ -1890,6 +1870,18 @@ static inline void mem_cgroup_unlock_pages(void)
 	rcu_read_unlock();
 }
 
+static inline void mem_cgroup_enter_user_fault(void)
+{
+	WARN_ON(current->in_user_fault);
+	current->in_user_fault = 1;
+}
+
+static inline void mem_cgroup_exit_user_fault(void)
+{
+	WARN_ON(!current->in_user_fault);
+	current->in_user_fault = 0;
+}
+
 #else /* CONFIG_MEMCG_V1 */
 static inline
 unsigned long memcg1_soft_limit_reclaim(pg_data_t *pgdat, int order,
@@ -1929,6 +1921,14 @@ static inline bool mem_cgroup_oom_synchronize(bool wait)
 	return false;
 }
 
+static inline void mem_cgroup_enter_user_fault(void)
+{
+}
+
+static inline void mem_cgroup_exit_user_fault(void)
+{
+}
+
 #endif /* CONFIG_MEMCG_V1 */
 
 #endif /* _LINUX_MEMCONTROL_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2a16023e8620..a7770c566c4d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -934,7 +934,7 @@ struct task_struct {
 #ifndef TIF_RESTORE_SIGMASK
 	unsigned			restore_sigmask:1;
 #endif
-#ifdef CONFIG_MEMCG
+#ifdef CONFIG_MEMCG_V1
 	unsigned			in_user_fault:1;
 #endif
 #ifdef CONFIG_LRU_GEN
-- 
2.45.2.803.g4e1b14247a-goog



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

* Re: [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c
  2024-06-28 21:03 ` [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c Roman Gushchin
@ 2024-06-29  0:30   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:30 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:09PM GMT, Roman Gushchin wrote:
> memcg_account_kmem() consists of a trivial statistics change via
> mod_memcg_state() call and a relatively large memcg1-specific part.
> 
> Let's factor out the mod_memcg_state() call and move the rest into
> the mm/memcontrol-v1.c file. Also rename memcg_account_kmem()
> into memcg1_account_kmem() for consistency.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code
  2024-06-28 21:03 ` [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code Roman Gushchin
@ 2024-06-29  0:39   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:39 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:10PM GMT, Roman Gushchin wrote:
> Move out the legacy cgroup v1 socket memory accounting code into
> mm/memcontrol-v1.c.
> 
> This commit introduces three new functions: memcg1_tcpmem_active(),
> memcg1_charge_skmem() and memcg1_uncharge_skmem(), which contain
> all cgroup v1-specific code and become trivial if CONFIG_MEMCG_V1
> isn't set.
> 
> Note, that !!memcg->tcpmem_pressure check in
> mem_cgroup_under_socket_pressure() can't be easily moved into
> memcontrol-v1.h without including memcontrol-v1.h from memcontrol.h
> which isn't a good idea, so it's better to just #ifdef it.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

[...]
> +static inline bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
> +				       gfp_t gfp_mask) { return true; }

I wonder if it really matters if we return true or false from above. If
CONFIG_MEMCG_V1=n then cgroup_subsys_on_dfl() is always true, so
memcg1_charge_skmem() should never be called. Anyways just wanted to
point this out, nothing actionable.

[...]
> +	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> +		return memcg1_charge_skmem(memcg, nr_pages, gfp_mask);


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

* Re: [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo()
  2024-06-28 21:03 ` [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo() Roman Gushchin
@ 2024-06-29  0:40   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:40 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:11PM GMT, Roman Gushchin wrote:
> Put cgroup v1-specific code in mem_cgroup_print_oom_meminfo() under
> CONFIG_MEMCG_V1.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>



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

* Re: [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init()
  2024-06-28 21:03 ` [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init() Roman Gushchin
@ 2024-06-29  0:43   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:43 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:12PM GMT, Roman Gushchin wrote:
> Gather all memcg1-specific struct mem_cgroup's members initialization
> in a new memcg1_memcg_init() function, defined in mm/memcontrol-v1.c.
> Obviously, if CONFIG_MEMCG_V1 is not set, there is no need to
> initialize these fields, so the function becomes trivial.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1
  2024-06-28 21:03 ` [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1 Roman Gushchin
@ 2024-06-29  0:48   ` Shakeel Butt
  2024-07-04 23:35     ` Andrew Morton
  2024-07-03 15:12   ` Shakeel Butt
  1 sibling, 1 reply; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:48 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:14PM GMT, Roman Gushchin wrote:
> Put memcg1-specific members of struct mem_cgroup under the
> CONFIG_MEMCG_V1 config option. Also group them close to the end
> of struct mem_cgroup just before the dynamic per-node part.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
> ---
>  include/linux/memcontrol.h | 103 +++++++++++++++++++------------------
>  1 file changed, 53 insertions(+), 50 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 44ab6394c9ed..107b0c5d6eab 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -188,10 +188,6 @@ struct mem_cgroup {
>  		struct page_counter memsw;	/* v1 only */
>  	};
>  
> -	/* Legacy consumer-oriented counters */
> -	struct page_counter kmem;		/* v1 only */
> -	struct page_counter tcpmem;		/* v1 only */
> -
>  	/* Range enforcement for interrupt charges */
>  	struct work_struct high_work;
>  
> @@ -205,8 +201,6 @@ struct mem_cgroup {
>  	bool zswap_writeback;
>  #endif
>  
> -	unsigned long soft_limit;
> -
>  	/* vmpressure notifications */
>  	struct vmpressure vmpressure;
>  
> @@ -215,13 +209,7 @@ struct mem_cgroup {
>  	 */
>  	bool oom_group;
>  
> -	/* protected by memcg_oom_lock */
> -	bool		oom_lock;
> -	int		under_oom;
> -
> -	int	swappiness;
> -	/* OOM-Killer disable */
> -	int		oom_kill_disable;
> +	int swappiness;
>  
>  	/* memory.events and memory.events.local */
>  	struct cgroup_file events_file;
> @@ -230,27 +218,6 @@ struct mem_cgroup {
>  	/* handle for "memory.swap.events" */
>  	struct cgroup_file swap_events_file;
>  
> -	/* protect arrays of thresholds */
> -	struct mutex thresholds_lock;
> -
> -	/* thresholds for memory usage. RCU-protected */
> -	struct mem_cgroup_thresholds thresholds;
> -
> -	/* thresholds for mem+swap usage. RCU-protected */
> -	struct mem_cgroup_thresholds memsw_thresholds;
> -
> -	/* For oom notifier event fd */
> -	struct list_head oom_notify;
> -
> -	/*
> -	 * Should we move charges of a task when a task is moved into this
> -	 * mem_cgroup ? And what type of charges should we move ?
> -	 */
> -	unsigned long move_charge_at_immigrate;
> -	/* taken only while moving_account > 0 */
> -	spinlock_t		move_lock;
> -	unsigned long		move_lock_flags;
> -
>  	CACHELINE_PADDING(_pad1_);

Let's also remove these _pad1_ and also _pad2_ as well as this
rearrangement nullifies the reasons behind these paddings. We need to
run some perf benchmarks to identify the newer false cache sharing
ields.


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

* Re: [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c
  2024-06-28 21:03 ` [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c Roman Gushchin
@ 2024-06-29  0:49   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:49 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:13PM GMT, Roman Gushchin wrote:
> There are only few memcg1-specific struct mem_cgroup's members
> accesses left in mm/memcontrol.c. Let's guard them with the
> CONFIG_MEMCG_V1 config option.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>



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

* Re: [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node
  2024-06-28 21:03 ` [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node Roman Gushchin
@ 2024-06-29  0:52   ` Shakeel Butt
  2024-07-03 15:13   ` Shakeel Butt
  1 sibling, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:52 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:15PM GMT, Roman Gushchin wrote:
> Put memcg1-specific members of struct mem_cgroup_per_node under the
> CONFIG_MEMCG_V1 config option.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
> ---
>  include/linux/memcontrol.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 107b0c5d6eab..c7ef628ee882 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -92,6 +92,7 @@ struct mem_cgroup_per_node {
>  	struct lruvec_stats			*lruvec_stats;
>  	struct shrinker_info __rcu	*shrinker_info;
>  
> +#ifdef CONFIG_MEMCG_V1
>  	/*
>  	 * Memcg-v1 only stuff in middle as buffer between read mostly fields
>  	 * and update often fields to avoid false sharing. Once v1 stuff is
> @@ -102,6 +103,7 @@ struct mem_cgroup_per_node {
>  	unsigned long		usage_in_excess;/* Set to the value by which */
>  						/* the soft limit is exceeded*/
>  	bool			on_tree;

Here we definitely need a padding after the pointers and before the
lruvec. We can add a #else to the #ifdef CONFIG_MEMCG_V1 and put a
cacheline padding in it.

> +#endif
>  
>  	/* Fields which get updated often at the end. */
>  	struct lruvec		lruvec;
> -- 
> 2.45.2.803.g4e1b14247a-goog
> 


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

* Re: [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1
  2024-06-28 21:03 ` [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1 Roman Gushchin
@ 2024-06-29  0:53   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:53 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:16PM GMT, Roman Gushchin wrote:
> The memcg_in_oom field of the struct task_struct is not used by
> the cgroup v2's memory controller, so it can be happily compiled out
> if CONFIG_MEMCG_V1 is not set.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault under CONFIG_MEMCG_V1
  2024-06-28 21:03 ` [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault " Roman Gushchin
@ 2024-06-29  0:55   ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-06-29  0:55 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:17PM GMT, Roman Gushchin wrote:
> The struct task_struct's in_user_fault member is not used by the
> cgroup v2's memory controller, so it can be put under the
> CONFIG_MEMCG_V1 config option. To do so, mem_cgroup_enter_user_fault()
> and mem_cgroup_exit_user_fault() are moved under the CONFIG_MEMCG_V1
> option as well.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1
  2024-06-28 21:03 ` [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1 Roman Gushchin
  2024-06-29  0:48   ` Shakeel Butt
@ 2024-07-03 15:12   ` Shakeel Butt
  1 sibling, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-07-03 15:12 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:14PM GMT, Roman Gushchin wrote:
> Put memcg1-specific members of struct mem_cgroup under the
> CONFIG_MEMCG_V1 config option. Also group them close to the end
> of struct mem_cgroup just before the dynamic per-node part.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node
  2024-06-28 21:03 ` [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node Roman Gushchin
  2024-06-29  0:52   ` Shakeel Butt
@ 2024-07-03 15:13   ` Shakeel Butt
  1 sibling, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-07-03 15:13 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, Jun 28, 2024 at 09:03:15PM GMT, Roman Gushchin wrote:
> Put memcg1-specific members of struct mem_cgroup_per_node under the
> CONFIG_MEMCG_V1 config option.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1
  2024-06-29  0:48   ` Shakeel Butt
@ 2024-07-04 23:35     ` Andrew Morton
  2024-07-05  3:43       ` Shakeel Butt
  0 siblings, 1 reply; 23+ messages in thread
From: Andrew Morton @ 2024-07-04 23:35 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Roman Gushchin, linux-mm, linux-kernel, Johannes Weiner,
	Michal Hocko, Muchun Song

On Fri, 28 Jun 2024 17:48:54 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:

> > -	/* For oom notifier event fd */
> > -	struct list_head oom_notify;
> > -
> > -	/*
> > -	 * Should we move charges of a task when a task is moved into this
> > -	 * mem_cgroup ? And what type of charges should we move ?
> > -	 */
> > -	unsigned long move_charge_at_immigrate;
> > -	/* taken only while moving_account > 0 */
> > -	spinlock_t		move_lock;
> > -	unsigned long		move_lock_flags;
> > -
> >  	CACHELINE_PADDING(_pad1_);
> 
> Let's also remove these _pad1_ and also _pad2_ as well as this
> rearrangement nullifies the reasons behind these paddings. We need to
> run some perf benchmarks to identify the newer false cache sharing
> ields.

I guess this is going to be a followup patch (please).


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

* Re: [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1
  2024-07-04 23:35     ` Andrew Morton
@ 2024-07-05  3:43       ` Shakeel Butt
  0 siblings, 0 replies; 23+ messages in thread
From: Shakeel Butt @ 2024-07-05  3:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Shakeel Butt, Roman Gushchin, linux-mm, linux-kernel,
	Johannes Weiner, Michal Hocko, Muchun Song

On Thu, Jul 4, 2024 at 4:35 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 28 Jun 2024 17:48:54 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> > > -   /* For oom notifier event fd */
> > > -   struct list_head oom_notify;
> > > -
> > > -   /*
> > > -    * Should we move charges of a task when a task is moved into this
> > > -    * mem_cgroup ? And what type of charges should we move ?
> > > -    */
> > > -   unsigned long move_charge_at_immigrate;
> > > -   /* taken only while moving_account > 0 */
> > > -   spinlock_t              move_lock;
> > > -   unsigned long           move_lock_flags;
> > > -
> > >     CACHELINE_PADDING(_pad1_);
> >
> > Let's also remove these _pad1_ and also _pad2_ as well as this
> > rearrangement nullifies the reasons behind these paddings. We need to
> > run some perf benchmarks to identify the newer false cache sharing
> > ields.
>
> I guess this is going to be a followup patch (please).

Already posted [1] and has been picked.

[1] https://lore.kernel.org/linux-mm/20240701185932.704807-1-roman.gushchin@linux.dev/


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

end of thread, other threads:[~2024-07-05  3:43 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-28 21:03 [PATCH v1 0/9] mm: memcg: put cgroup v1-specific memcg data under CONFIG_MEMCG_V1 Roman Gushchin
2024-06-28 21:03 ` [PATCH v1 1/9] mm: memcg: move memcg_account_kmem() to memcontrol-v1.c Roman Gushchin
2024-06-29  0:30   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code Roman Gushchin
2024-06-29  0:39   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 3/9] mm: memcg: guard cgroup v1-specific code in mem_cgroup_print_oom_meminfo() Roman Gushchin
2024-06-29  0:40   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 4/9] mm: memcg: gather memcg1-specific fields initialization in memcg1_memcg_init() Roman Gushchin
2024-06-29  0:43   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 5/9] mm: memcg: guard memcg1-specific fields accesses in mm/memcontrol.c Roman Gushchin
2024-06-29  0:49   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 6/9] mm: memcg: put memcg1-specific struct mem_cgroup's members under CONFIG_MEMCG_V1 Roman Gushchin
2024-06-29  0:48   ` Shakeel Butt
2024-07-04 23:35     ` Andrew Morton
2024-07-05  3:43       ` Shakeel Butt
2024-07-03 15:12   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 7/9] mm: memcg: guard memcg1-specific members of struct mem_cgroup_per_node Roman Gushchin
2024-06-29  0:52   ` Shakeel Butt
2024-07-03 15:13   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 8/9] mm: memcg: put struct task_struct::memcg_in_oom under CONFIG_MEMCG_V1 Roman Gushchin
2024-06-29  0:53   ` Shakeel Butt
2024-06-28 21:03 ` [PATCH v1 9/9] mm: memcg: put struct task_struct::in_user_fault " Roman Gushchin
2024-06-29  0:55   ` Shakeel Butt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox