linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][v6] x86/resctrl: Add task resctrl information display
@ 2020-01-10  7:06 Chen Yu
  2020-01-14  9:24 ` Borislav Petkov
  0 siblings, 1 reply; 3+ messages in thread
From: Chen Yu @ 2020-01-10  7:06 UTC (permalink / raw)
  To: x86
  Cc: Bhupesh Sharma, Dave Young, Aubrey Li, Mauro Carvalho Chehab,
	Krzysztof Kozlowski, Chen Yu, Thomas Gleixner, Borislav Petkov,
	Ingo Molnar, H. Peter Anvin, Alexey Dobriyan, Andrew Morton,
	Reinette Chatre, Fenghua Yu, Tony Luck, Chris Down, Michal Hocko,
	Linus Torvalds, linux-fsdevel, linux-kernel

Monitoring tools that want to find out which resctrl control
and monitor groups a task belongs to must currently read
the "tasks" file in every group until they locate the process
ID.

Add an additional file /proc/{pid}/cpu_resctrl to provide this
information.

The output is as followed, for example:

 1)   ""
      Resctrl is not available.

 2)   "/"
      Task is part of the root group, task is not associated to
      any monitor group.

 3)   "/mon_groups/mon0"
      Task is part of the root group and monitor group mon0.

 4)   "/group0"
      Task is part of resctrl control group group0, task is not
      associated to any monitor group.

 5)   "/group0/mon_groups/mon1"
      Task is part of resctrl control group group0 and monitor
      group mon1.

Tested-by: Jinshi Chen <jinshi.chen@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Chris Down <chris@chrisdown.name>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v1: Initial version reviewed by Reinette Chatre,
    Fenghua Yu and Tony Luck.

v2: According to Boris's suggestion,
    reduce indentation level in proc_resctrl_show().
    Create the include/linux/resctrl.h header and
    declare proc_resctrl_show() in this file, so
    that other architectures would probably use it
    in the future. Different architectures should
    implement architectural specific proc_resctrl_show()
    accordingly.

v3: According to Boris's suggestion,
    Return empty string if the resctrl filesystem has
    not been mounted.
    Rename the config from CPU_RESCTRL to PROC_CPU_RESCTRL
    to better represent its usage. Move PROC_CPU_RESCTRL
    from arch/Kconfig to fs/proc/Kconfig.
    And let PROC_CPU_RESCTRL to be depended on PROC_FS.

v4: According to Thomas's suggestion, changed the output
    from multiple lines to one single line.

v5: According to Alexey's feedback, removed the header file
    proc_fs.h in resctrl.h, and changed seq_puts() to
    seq_putc() for simplicity.

v6: According to Chris Down's suggestion,
    1. rename:
    /proc/{pid}/resctrl to /proc/{pid}/cpu_resctrl
    to better reflect its meaning.
    2. change the description in comments:
    "control group" to "resctrl control group"
    as the former is confusing for cgroup users.
---
 arch/x86/Kconfig                       |  1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 79 ++++++++++++++++++++++++++
 fs/proc/Kconfig                        |  4 ++
 fs/proc/base.c                         |  7 +++
 include/linux/resctrl.h                | 14 +++++
 5 files changed, 105 insertions(+)
 create mode 100644 include/linux/resctrl.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5e8949953660..6e17a68c7d77 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -456,6 +456,7 @@ config X86_CPU_RESCTRL
 	bool "x86 CPU resource control support"
 	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select KERNFS
+	select PROC_CPU_RESCTRL		if PROC_FS
 	help
 	  Enable x86 CPU resource control support.
 
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2e3b06d6bbc6..dcbf62d6b689 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -725,6 +725,85 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+#ifdef CONFIG_PROC_CPU_RESCTRL
+
+/*
+ * A task can only be part of one resctrl
+ * control group and of one monitor
+ * group which is associated to that resctrl
+ * control group.
+ * So one line is simple and clear enough:
+ *
+ * 1)   ""
+ *    resctrl is not available.
+ *
+ * 2)   "/"
+ *    Task is part of the root group, and it is
+ *    not associated to any monitor group.
+ *
+ * 3)   "/mon_groups/mon0"
+ *    Task is part of the root group and monitor
+ *    group mon0.
+ *
+ * 4)   "/group0"
+ *    Task is part of resctrl control group group0,
+ *    and it is not associated to any monitor group.
+ *
+ * 5)   "/group0/mon_groups/mon1"
+ *    Task is part of resctrl control group group0 and monitor
+ *    group mon1.
+ */
+int proc_resctrl_show(struct seq_file *s, struct pid_namespace *ns,
+		      struct pid *pid, struct task_struct *tsk)
+{
+	struct rdtgroup *rdtg;
+	int ret = 0;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	/* Return empty if resctrl has not been mounted. */
+	if (!static_branch_unlikely(&rdt_enable_key))
+		goto unlock;
+
+	list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
+		struct rdtgroup *crg;
+
+		/*
+		 * Task information is only relevant for shareable
+		 * and exclusive groups.
+		 */
+		if (rdtg->mode != RDT_MODE_SHAREABLE &&
+		    rdtg->mode != RDT_MODE_EXCLUSIVE)
+			continue;
+
+		if (rdtg->closid != tsk->closid)
+			continue;
+
+		seq_printf(s, "/%s", rdtg->kn->name);
+		list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
+				    mon.crdtgrp_list) {
+			if (tsk->rmid != crg->mon.rmid)
+				continue;
+			seq_printf(s, "%smon_groups/%s",
+				   rdtg == &rdtgroup_default ? "" : "/",
+				   crg->kn->name);
+			break;
+		}
+		seq_putc(s, '\n');
+		goto unlock;
+	}
+	/*
+	 * The above search should succeed. Otherwise return
+	 * with an error.
+	 */
+	ret = -ENOENT;
+unlock:
+	mutex_unlock(&rdtgroup_mutex);
+
+	return ret;
+}
+#endif
+
 static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
 				    struct seq_file *seq, void *v)
 {
diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
index 733881a6387b..27ef84d99f59 100644
--- a/fs/proc/Kconfig
+++ b/fs/proc/Kconfig
@@ -103,3 +103,7 @@ config PROC_CHILDREN
 config PROC_PID_ARCH_STATUS
 	def_bool n
 	depends on PROC_FS
+
+config PROC_CPU_RESCTRL
+	def_bool n
+	depends on PROC_FS
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea9501afb8..32c9ff154667 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -94,6 +94,7 @@
 #include <linux/sched/debug.h>
 #include <linux/sched/stat.h>
 #include <linux/posix-timers.h>
+#include <linux/resctrl.h>
 #include <trace/events/oom.h>
 #include "internal.h"
 #include "fd.h"
@@ -3060,6 +3061,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #endif
 #ifdef CONFIG_CGROUPS
 	ONE("cgroup",  S_IRUGO, proc_cgroup_show),
+#endif
+#ifdef CONFIG_PROC_CPU_RESCTRL
+	ONE("cpu_resctrl", S_IRUGO, proc_resctrl_show),
 #endif
 	ONE("oom_score",  S_IRUGO, proc_oom_score),
 	REG("oom_adj",    S_IRUGO|S_IWUSR, proc_oom_adj_operations),
@@ -3460,6 +3464,9 @@ static const struct pid_entry tid_base_stuff[] = {
 #endif
 #ifdef CONFIG_CGROUPS
 	ONE("cgroup",  S_IRUGO, proc_cgroup_show),
+#endif
+#ifdef CONFIG_PROC_CPU_RESCTRL
+	ONE("cpu_resctrl", S_IRUGO, proc_resctrl_show),
 #endif
 	ONE("oom_score", S_IRUGO, proc_oom_score),
 	REG("oom_adj",   S_IRUGO|S_IWUSR, proc_oom_adj_operations),
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
new file mode 100644
index 000000000000..daf5cf64c6a6
--- /dev/null
+++ b/include/linux/resctrl.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _RESCTRL_H
+#define _RESCTRL_H
+
+#ifdef CONFIG_PROC_CPU_RESCTRL
+
+int proc_resctrl_show(struct seq_file *m,
+		      struct pid_namespace *ns,
+		      struct pid *pid,
+		      struct task_struct *tsk);
+
+#endif
+
+#endif /* _RESCTRL_H */
-- 
2.17.1


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

* Re: [PATCH][v6] x86/resctrl: Add task resctrl information display
  2020-01-10  7:06 [PATCH][v6] x86/resctrl: Add task resctrl information display Chen Yu
@ 2020-01-14  9:24 ` Borislav Petkov
  2020-01-14 16:04   ` Chen Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Borislav Petkov @ 2020-01-14  9:24 UTC (permalink / raw)
  To: Chen Yu
  Cc: x86, Bhupesh Sharma, Dave Young, Aubrey Li,
	Mauro Carvalho Chehab, Krzysztof Kozlowski, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, Alexey Dobriyan, Andrew Morton,
	Reinette Chatre, Fenghua Yu, Tony Luck, Chris Down, Michal Hocko,
	Linus Torvalds, linux-fsdevel, linux-kernel

On Fri, Jan 10, 2020 at 03:06:08PM +0800, Chen Yu wrote:
> Monitoring tools that want to find out which resctrl control
> and monitor groups a task belongs to must currently read
> the "tasks" file in every group until they locate the process
> ID.
> 
> Add an additional file /proc/{pid}/cpu_resctrl to provide this
> information.
> 
> The output is as followed, for example:
> 
>  1)   ""
>       Resctrl is not available.
> 
>  2)   "/"
>       Task is part of the root group, task is not associated to
>       any monitor group.
> 
>  3)   "/mon_groups/mon0"
>       Task is part of the root group and monitor group mon0.
> 
>  4)   "/group0"
>       Task is part of resctrl control group group0, task is not
>       associated to any monitor group.
> 
>  5)   "/group0/mon_groups/mon1"
>       Task is part of resctrl control group group0 and monitor
>       group mon1.

So this way to present the information is totally non-intuitive,
IMNSVHO. What's wrong with:

1)
	res_group:
	mon_group:

2)
	res_group: /
	mon_group:

3)
	res_group: /
	mon_group: mon0

4)
	res_group: group0
	mon_group:

5)
	res_group: group0
	mon_group: mon1

?

You can even call the file "cpu_resctrl_groups" so that it is clear that
it will dump groups and then do:

	res: group0
	mon: mon1

which is both human-readable and easily greppable.

> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 2e3b06d6bbc6..dcbf62d6b689 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -725,6 +725,85 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_PROC_CPU_RESCTRL
> +
> +/*
> + * A task can only be part of one resctrl
> + * control group and of one monitor
> + * group which is associated to that resctrl
> + * control group.

Extend those comments to 80 cols.

> + * So one line is simple and clear enough:

Actually, the one line format you've done is confusing and can be done
much more human- and tool-readable.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH][v6] x86/resctrl: Add task resctrl information display
  2020-01-14  9:24 ` Borislav Petkov
@ 2020-01-14 16:04   ` Chen Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chen Yu @ 2020-01-14 16:04 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Chen Yu, the arch/x86 maintainers, Bhupesh Sharma, Dave Young,
	Aubrey Li, Mauro Carvalho Chehab, Krzysztof Kozlowski,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Alexey Dobriyan,
	Andrew Morton, Reinette Chatre, Fenghua Yu, Tony Luck,
	Chris Down, Michal Hocko, Linus Torvalds, linux-fsdevel,
	Linux Kernel Mailing List

Hi Boris,
On Tue, Jan 14, 2020 at 5:25 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Jan 10, 2020 at 03:06:08PM +0800, Chen Yu wrote:
> > Monitoring tools that want to find out which resctrl control
> > and monitor groups a task belongs to must currently read
> > the "tasks" file in every group until they locate the process
> > ID.
> >
> > Add an additional file /proc/{pid}/cpu_resctrl to provide this
> > information.
> >
> > The output is as followed, for example:
> >
> >  1)   ""
> >       Resctrl is not available.
> >
> >  2)   "/"
> >       Task is part of the root group, task is not associated to
> >       any monitor group.
> >
> >  3)   "/mon_groups/mon0"
> >       Task is part of the root group and monitor group mon0.
> >
> >  4)   "/group0"
> >       Task is part of resctrl control group group0, task is not
> >       associated to any monitor group.
> >
> >  5)   "/group0/mon_groups/mon1"
> >       Task is part of resctrl control group group0 and monitor
> >       group mon1.
>
> So this way to present the information is totally non-intuitive,
> IMNSVHO. What's wrong with:
>
> 1)
>         res_group:
>         mon_group:
>
> 2)
>         res_group: /
>         mon_group:
>
> 3)
>         res_group: /
>         mon_group: mon0
>
> 4)
>         res_group: group0
>         mon_group:
>
> 5)
>         res_group: group0
>         mon_group: mon1
>
> ?
>
> You can even call the file "cpu_resctrl_groups" so that it is clear that
> it will dump groups and then do:
>
>         res: group0
>         mon: mon1
>
> which is both human-readable and easily greppable.
>
Yes, to display resctrl control and monitor group separately might be more
friendly to the user.  Although I was thinking if the user would like
to see the full path of
the resource, which might make it easier to be parsed:
A) res: group0
    mon: mon1
vs
B) res: /group0
    mon: /group0/mon_groups/mon1
as proposal B might introduce duplication I'll send a new version
based on proposal A.
> > +/*
> > + * A task can only be part of one resctrl
> > + * control group and of one monitor
> > + * group which is associated to that resctrl
> > + * control group.
>
> Extend those comments to 80 cols.
>
Okay. will do.
> > + * So one line is simple and clear enough:
>
> Actually, the one line format you've done is confusing and can be done
> much more human- and tool-readable.
>
Got it.
Thanks,
Chenyu

> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

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

end of thread, other threads:[~2020-01-14 16:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10  7:06 [PATCH][v6] x86/resctrl: Add task resctrl information display Chen Yu
2020-01-14  9:24 ` Borislav Petkov
2020-01-14 16:04   ` Chen Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).