All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local
@ 2021-09-13  5:00 brookxu
  2021-09-13  5:01 ` [PATCH v2 2/3] misc_cgroup: remove error log to avoid log flood brookxu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: brookxu @ 2021-09-13  5:00 UTC (permalink / raw)
  To: tj, lizefan.x, hannes
  Cc: vipinsh, mkoutny, corbet, linux-kernel, cgroups, linux-doc

From: Chunguang Xu <brookxu@tencent.com>

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 <brookxu@tencent.com>
---
 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


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

* [PATCH v2 2/3] misc_cgroup: remove error log to avoid log flood
  2021-09-13  5:00 [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local brookxu
@ 2021-09-13  5:01 ` brookxu
  2021-09-13  5:01   ` brookxu
  2021-09-13 16:51 ` [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local Vipin Sharma
  2 siblings, 0 replies; 8+ messages in thread
From: brookxu @ 2021-09-13  5:01 UTC (permalink / raw)
  To: tj, lizefan.x, hannes
  Cc: vipinsh, mkoutny, corbet, linux-kernel, cgroups, linux-doc

From: Chunguang Xu <brookxu@tencent.com>

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 <brookxu@tencent.com>
---
 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


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

* [PATCH v2 3/3] docs/cgroup: add entry for misc.events and misc.events.local
@ 2021-09-13  5:01   ` brookxu
  0 siblings, 0 replies; 8+ messages in thread
From: brookxu @ 2021-09-13  5:01 UTC (permalink / raw)
  To: tj, lizefan.x, hannes
  Cc: vipinsh, mkoutny, corbet, linux-kernel, cgroups, linux-doc

From: Chunguang Xu <brookxu@tencent.com>

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

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 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


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

* [PATCH v2 3/3] docs/cgroup: add entry for misc.events and misc.events.local
@ 2021-09-13  5:01   ` brookxu
  0 siblings, 0 replies; 8+ messages in thread
From: brookxu @ 2021-09-13  5:01 UTC (permalink / raw)
  To: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan.x-EC8Uxl6Npydl57MIdRCFDg,
	hannes-druUgvl0LCNAfugRpC6u6w
  Cc: vipinsh-hpIqsD4AKlfQT0dZR+AlfA, mkoutny-IBi9RG/b67k,
	corbet-T1hC0tSOHrs, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA

From: Chunguang Xu <brookxu-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>

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

Signed-off-by: Chunguang Xu <brookxu-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
---
 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


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

* Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local
  2021-09-13  5:00 [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local brookxu
  2021-09-13  5:01 ` [PATCH v2 2/3] misc_cgroup: remove error log to avoid log flood brookxu
  2021-09-13  5:01   ` brookxu
@ 2021-09-13 16:51 ` Vipin Sharma
  2021-09-14  2:23   ` brookxu
  2 siblings, 1 reply; 8+ messages in thread
From: Vipin Sharma @ 2021-09-13 16:51 UTC (permalink / raw)
  To: brookxu
  Cc: tj, lizefan.x, hannes, mkoutny, corbet, linux-kernel, cgroups, linux-doc

On Sun, Sep 12, 2021 at 10:01 PM brookxu <brookxu.cn@gmail.com> wrote:
>
> From: Chunguang Xu <brookxu@tencent.com>
>
> 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],...

> @@ -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

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

* Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local
  2021-09-13 16:51 ` [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local Vipin Sharma
@ 2021-09-14  2:23   ` brookxu
  2021-09-14 20:10       ` Vipin Sharma
  0 siblings, 1 reply; 8+ messages in thread
From: brookxu @ 2021-09-14  2:23 UTC (permalink / raw)
  To: Vipin Sharma, Tejun Heo
  Cc: lizefan.x, hannes, mkoutny, corbet, linux-kernel, cgroups, linux-doc

Thanks for your time.

Vipin Sharma wrote on 2021/9/14 12:51 上午:
> On Sun, Sep 12, 2021 at 10:01 PM brookxu <brookxu.cn@gmail.com> wrote:
>>
>> From: Chunguang Xu <brookxu@tencent.com>
>>
>> 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?

>> 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],...
> 
>> @@ -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.

All right, thanks.

> Thanks
> Vipin
> 

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

* Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local
@ 2021-09-14 20:10       ` Vipin Sharma
  0 siblings, 0 replies; 8+ messages in thread
From: Vipin Sharma @ 2021-09-14 20:10 UTC (permalink / raw)
  To: brookxu
  Cc: Tejun Heo, lizefan.x, hannes, mkoutny, corbet, linux-kernel,
	cgroups, linux-doc

On Mon, Sep 13, 2021 at 7:24 PM brookxu <brookxu.cn@gmail.com> wrote:
>
> Thanks for your time.
>
> Vipin Sharma wrote on 2021/9/14 12:51 上午:
> > On Sun, Sep 12, 2021 at 10:01 PM brookxu <brookxu.cn@gmail.com> wrote:
> >>
> >> From: Chunguang Xu <brookxu@tencent.com>
> >>
> >> 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

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

* Re: [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local
@ 2021-09-14 20:10       ` Vipin Sharma
  0 siblings, 0 replies; 8+ messages in thread
From: Vipin Sharma @ 2021-09-14 20:10 UTC (permalink / raw)
  To: brookxu
  Cc: Tejun Heo, lizefan.x-EC8Uxl6Npydl57MIdRCFDg,
	hannes-druUgvl0LCNAfugRpC6u6w, mkoutny-IBi9RG/b67k,
	corbet-T1hC0tSOHrs, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA

On Mon, Sep 13, 2021 at 7:24 PM brookxu <brookxu.cn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Thanks for your time.
>
> Vipin Sharma wrote on 2021/9/14 12:51 上午:
> > On Sun, Sep 12, 2021 at 10:01 PM brookxu <brookxu.cn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>
> >> From: Chunguang Xu <brookxu-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
> >>
> >> 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-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.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

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

end of thread, other threads:[~2021-09-14 20:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13  5:00 [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local brookxu
2021-09-13  5:01 ` [PATCH v2 2/3] misc_cgroup: remove error log to avoid log flood brookxu
2021-09-13  5:01 ` [PATCH v2 3/3] docs/cgroup: add entry for misc.events and misc.events.local brookxu
2021-09-13  5:01   ` brookxu
2021-09-13 16:51 ` [PATCH v2 1/3] misc_cgroup: introduce misc.events and misc_events.local Vipin Sharma
2021-09-14  2:23   ` brookxu
2021-09-14 20:10     ` Vipin Sharma
2021-09-14 20:10       ` Vipin Sharma

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