* [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).