[PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local

Subsystems: control group (cgroup), the rest

STALE1792d

6 messages, 2 authors, 2021-09-14 · open the first message on its own page

[PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local

From: brookxu <hidden>
Date: 2021-09-13 05:01:10

From: Chunguang Xu <redacted>

Introduce misc.events and misc.events.local to make it easier for
us to understand the pressure of resources. The main idea comes
from mem_cgroup. Currently only the 'max' event is implemented,
which indicates the times the resource exceeds the limit.

Signed-off-by: Chunguang Xu <redacted>
---
 include/linux/misc_cgroup.h | 13 ++++++++++
 kernel/cgroup/misc.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
index da2367e..602fc11 100644
--- a/include/linux/misc_cgroup.h
+++ b/include/linux/misc_cgroup.h
@@ -21,6 +21,11 @@ enum misc_res_type {
 	MISC_CG_RES_TYPES
 };
 
+enum misc_event_type {
+	MISC_CG_EVENT_MAX,
+	MISC_CG_EVENT_TYPES
+};
+
 struct misc_cg;
 
 #ifdef CONFIG_CGROUP_MISC
@@ -36,6 +41,8 @@ enum misc_res_type {
 struct misc_res {
 	unsigned long max;
 	atomic_long_t usage;
+	atomic_long_t events[MISC_CG_EVENT_TYPES];
+	atomic_long_t events_local[MISC_CG_EVENT_TYPES];
 	bool failed;
 };
 
@@ -46,6 +53,12 @@ struct misc_res {
  */
 struct misc_cg {
 	struct cgroup_subsys_state css;
+
+	/* misc.events */
+	struct cgroup_file events_file;
+	/* misc.events.local */
+	struct cgroup_file events_local_file;
+
 	struct misc_res res[MISC_CG_RES_TYPES];
 };
 
diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
index ec02d96..5f06b2a 100644
--- a/kernel/cgroup/misc.c
+++ b/kernel/cgroup/misc.c
@@ -26,6 +26,10 @@
 #endif
 };
 
+static const char *const misc_event_name[] = {
+	"max"
+};
+
 /* Root misc cgroup */
 static struct misc_cg root_cg;
 
@@ -140,7 +144,7 @@ static void misc_cg_cancel_charge(enum misc_res_type type, struct misc_cg *cg,
 int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
 		       unsigned long amount)
 {
-	struct misc_cg *i, *j;
+	struct misc_cg *i, *j, *k;
 	int ret;
 	struct misc_res *res;
 	int new_usage;
@@ -171,6 +175,14 @@ int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
 	return 0;
 
 err_charge:
+	atomic_long_inc(&i->res[type].events_local[MISC_CG_EVENT_MAX]);
+	cgroup_file_notify(&i->events_local_file);
+
+	for (k = i; k; k = parent_misc(k)) {
+		atomic_long_inc(&k->res[type].events[MISC_CG_EVENT_MAX]);
+		cgroup_file_notify(&k->events_file);
+	}
+
 	for (j = cg; j != i; j = parent_misc(j))
 		misc_cg_cancel_charge(type, j, amount);
 	misc_cg_cancel_charge(type, i, amount);
@@ -335,6 +347,38 @@ static int misc_cg_capacity_show(struct seq_file *sf, void *v)
 	return 0;
 }
 
+static int misc_events_show(struct seq_file *sf, void *v)
+{
+	struct misc_cg *cg = css_misc(seq_css(sf));
+	unsigned long events, i, j;
+
+	for (i = 0; i < MISC_CG_RES_TYPES; i++) {
+		for (j = 0; j < MISC_CG_EVENT_TYPES; j++) {
+			events = atomic_long_read(&cg->res[i].events[j]);
+			if (READ_ONCE(misc_res_capacity[i]) || events)
+				seq_printf(sf, "%s.%s %lu\n", misc_res_name[i],
+					   misc_event_name[j], events);
+		}
+	}
+	return 0;
+}
+
+static int misc_events_local_show(struct seq_file *sf, void *v)
+{
+	struct misc_cg *cg = css_misc(seq_css(sf));
+	unsigned long events, i, j;
+
+	for (i = 0; i < MISC_CG_RES_TYPES; i++) {
+		for (j = 0; j < MISC_CG_EVENT_TYPES; j++) {
+			events = atomic_long_read(&cg->res[i].events_local[j]);
+			if (READ_ONCE(misc_res_capacity[i]) || events)
+				seq_printf(sf, "%s.%s %lu\n", misc_res_name[i],
+					   misc_event_name[j], events);
+		}
+	}
+	return 0;
+}
+
 /* Misc cgroup interface files */
 static struct cftype misc_cg_files[] = {
 	{
@@ -353,6 +397,18 @@ static int misc_cg_capacity_show(struct seq_file *sf, void *v)
 		.seq_show = misc_cg_capacity_show,
 		.flags = CFTYPE_ONLY_ON_ROOT,
 	},
+	{
+		.name = "events",
+		.flags = CFTYPE_NOT_ON_ROOT,
+		.file_offset = offsetof(struct misc_cg, events_file),
+		.seq_show = misc_events_show,
+	},
+	{
+		.name = "events.local",
+		.flags = CFTYPE_NOT_ON_ROOT,
+		.file_offset = offsetof(struct misc_cg, events_local_file),
+		.seq_show = misc_events_local_show,
+	},
 	{}
 };
 
-- 
1.8.3.1

[PATCH v2 2/3] misc_cgroup: remove error log to avoid log flood

From: brookxu <hidden>
Date: 2021-09-13 05:01:14

From: Chunguang Xu <redacted>

In scenarios where containers are frequently created and deleted,
a large number of error logs maybe generated. This log provides
less information, we can get more detailed info from misc.events,
misc.events.local. From this, perhaps we can remove it.

Signed-off-by: Chunguang Xu <redacted>
---
 include/linux/misc_cgroup.h | 1 -
 kernel/cgroup/misc.c        | 7 -------
 2 files changed, 8 deletions(-)
diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
index 602fc11..89dcb62 100644
--- a/include/linux/misc_cgroup.h
+++ b/include/linux/misc_cgroup.h
@@ -43,7 +43,6 @@ struct misc_res {
 	atomic_long_t usage;
 	atomic_long_t events[MISC_CG_EVENT_TYPES];
 	atomic_long_t events_local[MISC_CG_EVENT_TYPES];
-	bool failed;
 };
 
 /**
diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
index 5f06b2a..980ebaa 100644
--- a/kernel/cgroup/misc.c
+++ b/kernel/cgroup/misc.c
@@ -161,13 +161,6 @@ int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
 		new_usage = atomic_long_add_return(amount, &res->usage);
 		if (new_usage > READ_ONCE(res->max) ||
 		    new_usage > READ_ONCE(misc_res_capacity[type])) {
-			if (!res->failed) {
-				pr_info("cgroup: charge rejected by the misc controller for %s resource in ",
-					misc_res_name[type]);
-				pr_cont_cgroup_path(i->css.cgroup);
-				pr_cont("\n");
-				res->failed = true;
-			}
 			ret = -EBUSY;
 			goto err_charge;
 		}
-- 
1.8.3.1

[PATCH v2 3/3] docs/cgroup: add entry for misc.events and misc.events.local

From: brookxu <hidden>
Date: 2021-09-13 05:01:34

From: Chunguang Xu <redacted>

Added descriptions of misc.events and misc.events.local.

Signed-off-by: Chunguang Xu <redacted>
---
 Documentation/admin-guide/cgroup-v2.rst | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index babbe04..90a36ae 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2310,6 +2310,25 @@ Miscellaneous controller provides 3 interface files. If two misc resources (res_
         Limits can be set higher than the capacity value in the misc.capacity
         file.
 
+  misc.events
+	A read-only flat-keyed file which exists on non-root cgroups.
+	The following entries are defined.  Unless specified otherwise,
+	a value change in this file generates a file modified event.
+
+	Note that all fields in this file are hierarchical and the
+	file modified event can be generated due to an event down the
+	hierarchy. For the local events at the cgroup level see
+	misc.events.local.
+
+	  max
+		The number of times the cgroup's resource usage was
+		about to go over the max boundary.
+
+  misc.events.local
+	Similar to misc.events but the fields in the file are local
+	to the cgroup i.e. not hierarchical. The file modified event
+	generated on this file reflects only the local events.
+
 Migration and Ownership
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
1.8.3.1

Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local

From: Vipin Sharma <hidden>
Date: 2021-09-13 16:52:26

On Sun, Sep 12, 2021 at 10:01 PM brookxu [off-list ref] wrote:
From: Chunguang Xu <redacted>

Introduce misc.events and misc.events.local to make it easier for
I thought Tejun only gave go ahead for misc.events and not for
misc.events.local.
us to understand the pressure of resources. The main idea comes
from mem_cgroup. Currently only the 'max' event is implemented,
which indicates the times the resource exceeds the limit.
For future emails, please provide the links to previous discussions
like [1], [2],...
quoted hunk
@@ -36,6 +41,8 @@ enum misc_res_type {
 struct misc_res {
        unsigned long max;
        atomic_long_t usage;
+       atomic_long_t events[MISC_CG_EVENT_TYPES];
Since there is only one event type for now, my recommendation is to
not use the array and just use a single atomic_long_t.
+static const char *const misc_event_name[] = {
+       "max"
+};
+
We will not need it if you remove the array in struct misc_res.

Thanks
Vipin

Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local

From: brookxu <hidden>
Date: 2021-09-14 02:24:05

Thanks for your time.

Vipin Sharma wrote on 2021/9/14 12:51 上午:
On Sun, Sep 12, 2021 at 10:01 PM brookxu [off-list ref] wrote:
quoted
From: Chunguang Xu <redacted>

Introduce misc.events and misc.events.local to make it easier for
I thought Tejun only gave go ahead for misc.events and not for
misc.events.local.
Maybe I missed something. I think events.local is somewhat useful. For
example, the events of node A is large. If we need to determine whether
it is caused by the max of node A, if there is no events.local, then we
need to traverse the events of the child nodes and compare them with
node A. This is a bit complicated. If there is events.local, we can do
it very easily. Should we keep the events.local interface?
quoted
us to understand the pressure of resources. The main idea comes
from mem_cgroup. Currently only the 'max' event is implemented,
which indicates the times the resource exceeds the limit.
For future emails, please provide the links to previous discussions
like [1], [2],...
quoted
@@ -36,6 +41,8 @@ enum misc_res_type {
 struct misc_res {
        unsigned long max;
        atomic_long_t usage;
+       atomic_long_t events[MISC_CG_EVENT_TYPES];
Since there is only one event type for now, my recommendation is to
not use the array and just use a single atomic_long_t.
quoted
+static const char *const misc_event_name[] = {
+       "max"
+};
+
We will not need it if you remove the array in struct misc_res.
All right, thanks.
Thanks
Vipin

Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local

From: Vipin Sharma <hidden>
Date: 2021-09-14 20:11:09

On Mon, Sep 13, 2021 at 7:24 PM brookxu [off-list ref] wrote:
Thanks for your time.

Vipin Sharma wrote on 2021/9/14 12:51 上午:
quoted
On Sun, Sep 12, 2021 at 10:01 PM brookxu [off-list ref] wrote:
quoted
From: Chunguang Xu <redacted>

Introduce misc.events and misc.events.local to make it easier for
I thought Tejun only gave go ahead for misc.events and not for
misc.events.local.
Maybe I missed something. I think events.local is somewhat useful. For
example, the events of node A is large. If we need to determine whether
it is caused by the max of node A, if there is no events.local, then we
need to traverse the events of the child nodes and compare them with
node A. This is a bit complicated. If there is events.local, we can do
it very easily. Should we keep the events.local interface?
Tejun mentioned in his previous email that he prefers the hierarchical
one. https://lore.kernel.org/lkml/YTuX6Cpv1kg+DHmJ@slm.duckdns.org/

I agree with you that it's easier to identify the constraint cgroup
with the local file. However, there is one downside also, which is if
a cgroup gets deleted then that local information is lost, we will
need a hierarchical reporting to observe the resource constraint. I
will be fine with both files but if I have to choose one I am now more
inclined towards hierarchical (events).

Thanks
Vipin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help