* [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
@ 2017-03-29 15:09 Jiri Olsa
2017-03-29 16:08 ` Fenghua Yu
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-03-29 15:09 UTC (permalink / raw)
To: Fenghua Yu
Cc: Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml, Ingo Molnar,
Peter Zijlstra, Thomas Gleixner
While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.
Adding cpus_list file to provide cpumask list interface
to define group's cpus.
# cd /sys/fs/resctrl/
# echo 1-10 > krava/cpus_list
# cat krava/cpus_list
1-10
# cat krava/cpus
0007fe
# cat cpus
fffff9
# cat cpus_list
0,3-23
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/include/asm/intel_rdt.h | 16 +++++++++---
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 42 +++++++++++++++++++++++---------
arch/x86/kernel/cpu/intel_rdt_schemata.c | 6 +++--
3 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 0d64397cee58..0e74e58e1f23 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
/* rdtgroup.flags */
#define RDT_DELETED 1
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST 1
+
/* List of all resource groups */
extern struct list_head rdt_all_groups;
@@ -54,16 +57,19 @@ struct rftype {
char *name;
umode_t mode;
struct kernfs_ops *kf_ops;
+ unsigned long flags;
int (*seq_show)(struct kernfs_open_file *of,
- struct seq_file *sf, void *v);
+ struct seq_file *sf, void *v,
+ unsigned long flags);
/*
* write() is the generic write callback which maps directly to
* kernfs write operation and overrides all other operations.
* Maximum write size is determined by ->max_write_len.
*/
ssize_t (*write)(struct kernfs_open_file *of,
- char *buf, size_t nbytes, loff_t off);
+ char *buf, size_t nbytes, loff_t off,
+ unsigned long flags);
};
/**
@@ -179,9 +185,11 @@ void rdt_cbm_update(void *arg);
struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn);
void rdtgroup_kn_unlock(struct kernfs_node *kn);
ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
- char *buf, size_t nbytes, loff_t off);
+ char *buf, size_t nbytes, loff_t off,
+ unsigned long flags);
int rdtgroup_schemata_show(struct kernfs_open_file *of,
- struct seq_file *s, void *v);
+ struct seq_file *s, void *v,
+ unsigned long flags);
/*
* intel_rdt_sched_in() - Writes the task's CLOSid to IA32_PQR_MSR
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..3a1dfb421dc5 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -153,7 +153,7 @@ static int rdtgroup_seqfile_show(struct seq_file *m, void *arg)
struct rftype *rft = of->kn->priv;
if (rft->seq_show)
- return rft->seq_show(of, m, arg);
+ return rft->seq_show(of, m, arg, rft->flags);
return 0;
}
@@ -163,7 +163,7 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
struct rftype *rft = of->kn->priv;
if (rft->write)
- return rft->write(of, buf, nbytes, off);
+ return rft->write(of, buf, nbytes, off, rft->flags);
return -EINVAL;
}
@@ -175,15 +175,17 @@ static struct kernfs_ops rdtgroup_kf_single_ops = {
};
static int rdtgroup_cpus_show(struct kernfs_open_file *of,
- struct seq_file *s, void *v)
+ struct seq_file *s, void *v,
+ unsigned long flags)
{
+ const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
struct rdtgroup *rdtgrp;
int ret = 0;
rdtgrp = rdtgroup_kn_lock_live(of->kn);
if (rdtgrp)
- seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
+ seq_printf(s, fmt, cpumask_pr_args(&rdtgrp->cpu_mask));
else
ret = -ENOENT;
rdtgroup_kn_unlock(of->kn);
@@ -230,7 +232,8 @@ rdt_update_closid(const struct cpumask *cpu_mask, int *closid)
}
static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
- char *buf, size_t nbytes, loff_t off)
+ char *buf, size_t nbytes, loff_t off,
+ unsigned long flags)
{
cpumask_var_t tmpmask, newmask;
struct rdtgroup *rdtgrp, *r;
@@ -252,7 +255,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = cpumask_parse(buf, newmask);
+ if (flags & RFTYPE_FLAGS_CPUS_LIST)
+ ret = cpulist_parse(buf, newmask);
+ else
+ ret = cpumask_parse(buf, newmask);
+
if (ret)
goto unlock;
@@ -415,7 +422,8 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
}
static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
- char *buf, size_t nbytes, loff_t off)
+ char *buf, size_t nbytes, loff_t off,
+ unsigned long flags __maybe_unused)
{
struct rdtgroup *rdtgrp;
int ret = 0;
@@ -448,7 +456,8 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
}
static int rdtgroup_tasks_show(struct kernfs_open_file *of,
- struct seq_file *s, void *v)
+ struct seq_file *s, void *v,
+ unsigned long flags)
{
struct rdtgroup *rdtgrp;
int ret = 0;
@@ -473,6 +482,14 @@ static struct rftype rdtgroup_base_files[] = {
.seq_show = rdtgroup_cpus_show,
},
{
+ .name = "cpus_list",
+ .mode = 0644,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .write = rdtgroup_cpus_write,
+ .seq_show = rdtgroup_cpus_show,
+ .flags = RFTYPE_FLAGS_CPUS_LIST,
+ },
+ {
.name = "tasks",
.mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
@@ -489,7 +506,8 @@ static struct rftype rdtgroup_base_files[] = {
};
static int rdt_num_closids_show(struct kernfs_open_file *of,
- struct seq_file *seq, void *v)
+ struct seq_file *seq, void *v,
+ unsigned long flags __maybe_unused)
{
struct rdt_resource *r = of->kn->parent->priv;
@@ -499,7 +517,8 @@ static int rdt_num_closids_show(struct kernfs_open_file *of,
}
static int rdt_cbm_mask_show(struct kernfs_open_file *of,
- struct seq_file *seq, void *v)
+ struct seq_file *seq, void *v,
+ unsigned long flags __maybe_unused)
{
struct rdt_resource *r = of->kn->parent->priv;
@@ -509,7 +528,8 @@ static int rdt_cbm_mask_show(struct kernfs_open_file *of,
}
static int rdt_min_cbm_bits_show(struct kernfs_open_file *of,
- struct seq_file *seq, void *v)
+ struct seq_file *seq, void *v,
+ unsigned long flags __maybe_unused)
{
struct rdt_resource *r = of->kn->parent->priv;
diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c
index f369cb8db0d5..3780ebebf36c 100644
--- a/arch/x86/kernel/cpu/intel_rdt_schemata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c
@@ -132,7 +132,8 @@ static int update_domains(struct rdt_resource *r, int closid)
}
ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
- char *buf, size_t nbytes, loff_t off)
+ char *buf, size_t nbytes, loff_t off,
+ unsigned long flags)
{
struct rdtgroup *rdtgrp;
struct rdt_resource *r;
@@ -224,7 +225,8 @@ static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
}
int rdtgroup_schemata_show(struct kernfs_open_file *of,
- struct seq_file *s, void *v)
+ struct seq_file *s, void *v,
+ unsigned long flags __maybe_unused)
{
struct rdtgroup *rdtgrp;
struct rdt_resource *r;
--
2.9.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-29 15:09 [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa
@ 2017-03-29 16:08 ` Fenghua Yu
2017-03-29 16:16 ` Jiri Olsa
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Fenghua Yu @ 2017-03-29 16:08 UTC (permalink / raw)
To: Jiri Olsa
Cc: Fenghua Yu, Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml,
Ingo Molnar, Peter Zijlstra, Thomas Gleixner
On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> While playing with the resctrl interface I found it much
> easier to deal with cpumask list rather than just regular
> cpumask.
Could you please explain specifically why and when it's easier
to deal with cpumask list? In programming cases, cpumask
and cpumask list are almost same. And people are working
on higher level tools to control resctrl. The tools can
hide detailed regular cpumask or cpumask list and user
doesn't need to care lower level format of cpumask. So
is it really useful to add cpus_list?
>
> Adding cpus_list file to provide cpumask list interface
> to define group's cpus.
>
> # cd /sys/fs/resctrl/
> # echo 1-10 > krava/cpus_list
> # cat krava/cpus_list
> 1-10
> # cat krava/cpus
> 0007fe
> # cat cpus
> fffff9
> # cat cpus_list
> 0,3-23
>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> arch/x86/include/asm/intel_rdt.h | 16 +++++++++---
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 42 +++++++++++++++++++++++---------
> arch/x86/kernel/cpu/intel_rdt_schemata.c | 6 +++--
> 3 files changed, 47 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
> index 0d64397cee58..0e74e58e1f23 100644
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -37,6 +37,9 @@ struct rdtgroup {
> /* rdtgroup.flags */
> #define RDT_DELETED 1
>
> +/* rftype.flags */
> +#define RFTYPE_FLAGS_CPUS_LIST 1
> +
> /* List of all resource groups */
> extern struct list_head rdt_all_groups;
>
> @@ -54,16 +57,19 @@ struct rftype {
> char *name;
> umode_t mode;
> struct kernfs_ops *kf_ops;
> + unsigned long flags;
>
> int (*seq_show)(struct kernfs_open_file *of,
> - struct seq_file *sf, void *v);
> + struct seq_file *sf, void *v,
> + unsigned long flags);
> /*
> * write() is the generic write callback which maps directly to
> * kernfs write operation and overrides all other operations.
> * Maximum write size is determined by ->max_write_len.
> */
> ssize_t (*write)(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off);
> + char *buf, size_t nbytes, loff_t off,
> + unsigned long flags);
I think no need to use "flags". Withou adding this "flags", the patch
will be much concise.
> };
>
> /**
> @@ -179,9 +185,11 @@ void rdt_cbm_update(void *arg);
> struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn);
> void rdtgroup_kn_unlock(struct kernfs_node *kn);
> ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off);
> + char *buf, size_t nbytes, loff_t off,
> + unsigned long flags);
> int rdtgroup_schemata_show(struct kernfs_open_file *of,
> - struct seq_file *s, void *v);
> + struct seq_file *s, void *v,
> + unsigned long flags);
No need for this hunk.
>
> /*
> * intel_rdt_sched_in() - Writes the task's CLOSid to IA32_PQR_MSR
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 9ac2a5cdd9c2..3a1dfb421dc5 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -153,7 +153,7 @@ static int rdtgroup_seqfile_show(struct seq_file *m, void *arg)
> struct rftype *rft = of->kn->priv;
>
> if (rft->seq_show)
> - return rft->seq_show(of, m, arg);
> + return rft->seq_show(of, m, arg, rft->flags);
> }
Ditto.
>
> @@ -163,7 +163,7 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
> struct rftype *rft = of->kn->priv;
>
> if (rft->write)
> - return rft->write(of, buf, nbytes, off);
> + return rft->write(of, buf, nbytes, off, rft->flags);
>
> return -EINVAL;
> }
Ditto.
> @@ -175,15 +175,17 @@ static struct kernfs_ops rdtgroup_kf_single_ops = {
> };
>
> static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> - struct seq_file *s, void *v)
> + struct seq_file *s, void *v,
> + unsigned long flags)
> {
> + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
Change to:
+ const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> struct rdtgroup *rdtgrp;
> int ret = 0;
>
> rdtgrp = rdtgroup_kn_lock_live(of->kn);
>
> if (rdtgrp)
> - seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> + seq_printf(s, fmt, cpumask_pr_args(&rdtgrp->cpu_mask));
> else
> ret = -ENOENT;
> rdtgroup_kn_unlock(of->kn);
> @@ -230,7 +232,8 @@ rdt_update_closid(const struct cpumask *cpu_mask, int *closid)
> }
>
> static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off)
> + char *buf, size_t nbytes, loff_t off,
> + unsigned long flags)
> {
> cpumask_var_t tmpmask, newmask;
> struct rdtgroup *rdtgrp, *r;
No need for this hunk.
> @@ -252,7 +255,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
> goto unlock;
> }
>
> - ret = cpumask_parse(buf, newmask);
> + if (flags & RFTYPE_FLAGS_CPUS_LIST)
Change to:
+ if (strcmp(of->kn->priv, "cpus"))
> + ret = cpulist_parse(buf, newmask);
> + else
> + ret = cpumask_parse(buf, newmask);
> +
> if (ret)
> goto unlock;
>
> @@ -415,7 +422,8 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
> }
>
> static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off)
> + char *buf, size_t nbytes, loff_t off,
> + unsigned long flags __maybe_unused)
> {
> struct rdtgroup *rdtgrp;
> int ret = 0;
No need for this hunk.
> @@ -448,7 +456,8 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
> }
>
> static int rdtgroup_tasks_show(struct kernfs_open_file *of,
> - struct seq_file *s, void *v)
> + struct seq_file *s, void *v,
> + unsigned long flags)
> {
> struct rdtgroup *rdtgrp;
> int ret = 0;
Ditto.
> @@ -473,6 +482,14 @@ static struct rftype rdtgroup_base_files[] = {
> .seq_show = rdtgroup_cpus_show,
> },
> {
> + .name = "cpus_list",
> + .mode = 0644,
> + .kf_ops = &rdtgroup_kf_single_ops,
> + .write = rdtgroup_cpus_write,
> + .seq_show = rdtgroup_cpus_show,
> + .flags = RFTYPE_FLAGS_CPUS_LIST,
> + },
> + {
> .name = "tasks",
> .mode = 0644,
> .kf_ops = &rdtgroup_kf_single_ops,
No need for the rest of the patch.
> @@ -489,7 +506,8 @@ static struct rftype rdtgroup_base_files[] = {
> };
>
> static int rdt_num_closids_show(struct kernfs_open_file *of,
> - struct seq_file *seq, void *v)
> + struct seq_file *seq, void *v,
> + unsigned long flags __maybe_unused)
> {
> struct rdt_resource *r = of->kn->parent->priv;
>
> @@ -499,7 +517,8 @@ static int rdt_num_closids_show(struct kernfs_open_file *of,
> }
>
> static int rdt_cbm_mask_show(struct kernfs_open_file *of,
> - struct seq_file *seq, void *v)
> + struct seq_file *seq, void *v,
> + unsigned long flags __maybe_unused)
> {
> struct rdt_resource *r = of->kn->parent->priv;
>
> @@ -509,7 +528,8 @@ static int rdt_cbm_mask_show(struct kernfs_open_file *of,
> }
>
> static int rdt_min_cbm_bits_show(struct kernfs_open_file *of,
> - struct seq_file *seq, void *v)
> + struct seq_file *seq, void *v,
> + unsigned long flags __maybe_unused)
> {
> struct rdt_resource *r = of->kn->parent->priv;
>
> diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c
> index f369cb8db0d5..3780ebebf36c 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c
> @@ -132,7 +132,8 @@ static int update_domains(struct rdt_resource *r, int closid)
> }
>
> ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off)
> + char *buf, size_t nbytes, loff_t off,
> + unsigned long flags)
> {
> struct rdtgroup *rdtgrp;
> struct rdt_resource *r;
> @@ -224,7 +225,8 @@ static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
> }
>
> int rdtgroup_schemata_show(struct kernfs_open_file *of,
> - struct seq_file *s, void *v)
> + struct seq_file *s, void *v,
> + unsigned long flags __maybe_unused)
> {
> struct rdtgroup *rdtgrp;
> struct rdt_resource *r;
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-29 16:08 ` Fenghua Yu
@ 2017-03-29 16:16 ` Jiri Olsa
2017-03-30 14:59 ` Jiri Olsa
2017-03-31 8:47 ` Thomas Gleixner
2017-03-31 9:15 ` Thomas Gleixner
2 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-03-29 16:16 UTC (permalink / raw)
To: Fenghua Yu
Cc: Jiri Olsa, Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml,
Ingo Molnar, Peter Zijlstra, Thomas Gleixner
On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > While playing with the resctrl interface I found it much
> > easier to deal with cpumask list rather than just regular
> > cpumask.
>
> Could you please explain specifically why and when it's easier
> to deal with cpumask list? In programming cases, cpumask
> and cpumask list are almost same. And people are working
> on higher level tools to control resctrl. The tools can
> hide detailed regular cpumask or cpumask list and user
> doesn't need to care lower level format of cpumask. So
> is it really useful to add cpus_list?
well I'm not aware about any such tool so I used resctrl
interface directly, and in that case it was much simpler
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-29 16:16 ` Jiri Olsa
@ 2017-03-30 14:59 ` Jiri Olsa
2017-04-01 2:00 ` Fenghua Yu
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-03-30 14:59 UTC (permalink / raw)
To: Fenghua Yu
Cc: Jiri Olsa, Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml,
Ingo Molnar, Peter Zijlstra, Thomas Gleixner
On Wed, Mar 29, 2017 at 06:16:00PM +0200, Jiri Olsa wrote:
> On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > While playing with the resctrl interface I found it much
> > > easier to deal with cpumask list rather than just regular
> > > cpumask.
> >
> > Could you please explain specifically why and when it's easier
> > to deal with cpumask list? In programming cases, cpumask
> > and cpumask list are almost same. And people are working
> > on higher level tools to control resctrl. The tools can
> > hide detailed regular cpumask or cpumask list and user
> > doesn't need to care lower level format of cpumask. So
> > is it really useful to add cpus_list?
>
>
> well I'm not aware about any such tool so I used resctrl
> interface directly, and in that case it was much simpler
is there any tool available for this actualy?
thanks,
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-29 16:08 ` Fenghua Yu
2017-03-29 16:16 ` Jiri Olsa
@ 2017-03-31 8:47 ` Thomas Gleixner
2017-03-31 15:53 ` Fenghua Yu
2017-03-31 9:15 ` Thomas Gleixner
2 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2017-03-31 8:47 UTC (permalink / raw)
To: Fenghua Yu
Cc: Jiri Olsa, Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml,
Ingo Molnar, Peter Zijlstra
On Wed, 29 Mar 2017, Fenghua Yu wrote:
> On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > While playing with the resctrl interface I found it much
> > easier to deal with cpumask list rather than just regular
> > cpumask.
>
> Could you please explain specifically why and when it's easier
> to deal with cpumask list? In programming cases, cpumask
> and cpumask list are almost same. And people are working
> on higher level tools to control resctrl. The tools can
> hide detailed regular cpumask or cpumask list and user
> doesn't need to care lower level format of cpumask. So
> is it really useful to add cpus_list?
Yes, because a lot of people including me do not care about these tools at
all. Making it easy to read and write from the command line is the first
thing to do.
Thanks,
tglx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-29 16:08 ` Fenghua Yu
2017-03-29 16:16 ` Jiri Olsa
2017-03-31 8:47 ` Thomas Gleixner
@ 2017-03-31 9:15 ` Thomas Gleixner
2017-03-31 15:40 ` Fenghua Yu
2017-03-31 15:55 ` Jiri Olsa
2 siblings, 2 replies; 17+ messages in thread
From: Thomas Gleixner @ 2017-03-31 9:15 UTC (permalink / raw)
To: Fenghua Yu
Cc: Jiri Olsa, Peter Zijlstra, Mike Galbraith, Shaohua Li, lkml,
Ingo Molnar, Peter Zijlstra
On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > - struct seq_file *s, void *v)
> > + struct seq_file *s, void *v,
> > + unsigned long flags)
> > {
> > + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
>
> Change to:
> + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
You couldn't come up with a more horrible hack, right?
Jiri was right with adding the flag to the base files, just the propagation
through the callbacks sucks. What's wrong with:
struct rftype *rft = of->kn->priv;
bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
Thanks,
tglx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-31 9:15 ` Thomas Gleixner
@ 2017-03-31 15:40 ` Fenghua Yu
2017-03-31 15:55 ` Jiri Olsa
1 sibling, 0 replies; 17+ messages in thread
From: Fenghua Yu @ 2017-03-31 15:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > - struct seq_file *s, void *v)
> > > + struct seq_file *s, void *v,
> > > + unsigned long flags)
> > > {
> > > + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> >
> > Change to:
> > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
>
> You couldn't come up with a more horrible hack, right?
>
> Jiri was right with adding the flag to the base files, just the propagation
> through the callbacks sucks. What's wrong with:
>
> struct rftype *rft = of->kn->priv;
> bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
You are absolutely correct.
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-31 8:47 ` Thomas Gleixner
@ 2017-03-31 15:53 ` Fenghua Yu
0 siblings, 0 replies; 17+ messages in thread
From: Fenghua Yu @ 2017-03-31 15:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Fri, Mar 31, 2017 at 10:47:52AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > While playing with the resctrl interface I found it much
> > > easier to deal with cpumask list rather than just regular
> > > cpumask.
> >
> > Could you please explain specifically why and when it's easier
> > to deal with cpumask list? In programming cases, cpumask
> > and cpumask list are almost same. And people are working
> > on higher level tools to control resctrl. The tools can
> > hide detailed regular cpumask or cpumask list and user
> > doesn't need to care lower level format of cpumask. So
> > is it really useful to add cpus_list?
>
> Yes, because a lot of people including me do not care about these tools at
> all. Making it easy to read and write from the command line is the first
> thing to do.
Sure, you are right. Hopefully adding a bit more explanation in
the commit message will be helpful.
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-31 9:15 ` Thomas Gleixner
2017-03-31 15:40 ` Fenghua Yu
@ 2017-03-31 15:55 ` Jiri Olsa
2017-04-09 16:49 ` [PATCHv2] " Jiri Olsa
1 sibling, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-03-31 15:55 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > - struct seq_file *s, void *v)
> > > + struct seq_file *s, void *v,
> > > + unsigned long flags)
> > > {
> > > + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> >
> > Change to:
> > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
>
> You couldn't come up with a more horrible hack, right?
>
> Jiri was right with adding the flag to the base files, just the propagation
> through the callbacks sucks. What's wrong with:
>
> struct rftype *rft = of->kn->priv;
> bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
ok, I'll post the change
thanks,
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-30 14:59 ` Jiri Olsa
@ 2017-04-01 2:00 ` Fenghua Yu
0 siblings, 0 replies; 17+ messages in thread
From: Fenghua Yu @ 2017-04-01 2:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra, Thomas Gleixner
On Thu, Mar 30, 2017 at 04:59:47PM +0200, Jiri Olsa wrote:
> On Wed, Mar 29, 2017 at 06:16:00PM +0200, Jiri Olsa wrote:
> > On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> > > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > > While playing with the resctrl interface I found it much
> > > > easier to deal with cpumask list rather than just regular
> > > > cpumask.
> > >
> > > Could you please explain specifically why and when it's easier
> > > to deal with cpumask list? In programming cases, cpumask
> > > and cpumask list are almost same. And people are working
> > > on higher level tools to control resctrl. The tools can
> > > hide detailed regular cpumask or cpumask list and user
> > > doesn't need to care lower level format of cpumask. So
> > > is it really useful to add cpus_list?
> >
> >
> > well I'm not aware about any such tool so I used resctrl
> > interface directly, and in that case it was much simpler
>
> is there any tool available for this actualy?
Here is a tool created by Marcelo:
https://lwn.net/Articles/710514/
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file
2017-03-31 15:55 ` Jiri Olsa
@ 2017-04-09 16:49 ` Jiri Olsa
2017-04-09 17:38 ` Fenghua Yu
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-04-09 16:49 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Fri, Mar 31, 2017 at 05:55:12PM +0200, Jiri Olsa wrote:
> On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> > On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > > static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > > - struct seq_file *s, void *v)
> > > > + struct seq_file *s, void *v,
> > > > + unsigned long flags)
> > > > {
> > > > + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > >
> > > Change to:
> > > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> >
> > You couldn't come up with a more horrible hack, right?
> >
> > Jiri was right with adding the flag to the base files, just the propagation
> > through the callbacks sucks. What's wrong with:
> >
> > struct rftype *rft = of->kn->priv;
> > bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
>
> ok, I'll post the change
v2 attached
thanks,
jirka
---
While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.
Adding cpus_list file to provide cpumask list interface
to define group's cpus.
# cd /sys/fs/resctrl/
# echo 1-10 > krava/cpus_list
# cat krava/cpus_list
1-10
# cat krava/cpus
0007fe
# cat cpus
fffff9
# cat cpus_list
0,3-23
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/include/asm/intel_rdt.h | 4 ++++
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..a1a6681738a5 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
/* rdtgroup.flags */
#define RDT_DELETED 1
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST 1
+
/* List of all resource groups */
extern struct list_head rdt_all_groups;
@@ -56,6 +59,7 @@ struct rftype {
char *name;
umode_t mode;
struct kernfs_ops *kf_ops;
+ unsigned long flags;
int (*seq_show)(struct kernfs_open_file *of,
struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..b089d2452f1e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
.seq_show = rdtgroup_seqfile_show,
};
+static bool is_list(struct kernfs_open_file *of)
+{
+ struct rftype *rft = of->kn->priv;
+
+ return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
static int rdtgroup_cpus_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
rdtgrp = rdtgroup_kn_lock_live(of->kn);
- if (rdtgrp)
- seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
- else
+ if (rdtgrp) {
+ seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+ cpumask_pr_args(&rdtgrp->cpu_mask));
+ } else
ret = -ENOENT;
rdtgroup_kn_unlock(of->kn);
@@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = cpumask_parse(buf, newmask);
+ if (is_list(of))
+ ret = cpulist_parse(buf, newmask);
+ else
+ ret = cpumask_parse(buf, newmask);
+
if (ret)
goto unlock;
@@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
.seq_show = rdtgroup_cpus_show,
},
{
+ .name = "cpus_list",
+ .mode = 0644,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .write = rdtgroup_cpus_write,
+ .seq_show = rdtgroup_cpus_show,
+ .flags = RFTYPE_FLAGS_CPUS_LIST,
+ },
+ {
.name = "tasks",
.mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-09 16:49 ` [PATCHv2] " Jiri Olsa
@ 2017-04-09 17:38 ` Fenghua Yu
2017-04-10 7:40 ` [PATCHv3] " Jiri Olsa
2017-04-10 9:14 ` [PATCHv2] " Thomas Gleixner
0 siblings, 2 replies; 17+ messages in thread
From: Fenghua Yu @ 2017-04-09 17:38 UTC (permalink / raw)
To: Jiri Olsa
Cc: Thomas Gleixner, Fenghua Yu, Jiri Olsa, Peter Zijlstra,
Mike Galbraith, Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Sun, Apr 09, 2017 at 06:49:29PM +0200, Jiri Olsa wrote:
> On Fri, Mar 31, 2017 at 05:55:12PM +0200, Jiri Olsa wrote:
> > On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> > > On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > > > static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > > > - struct seq_file *s, void *v)
> > > > > + struct seq_file *s, void *v,
> > > > > + unsigned long flags)
> > > > > {
> > > > > + const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > > >
> > > > Change to:
> > > > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> > >
> > > You couldn't come up with a more horrible hack, right?
> > >
> > > Jiri was right with adding the flag to the base files, just the propagation
> > > through the callbacks sucks. What's wrong with:
> > >
> > > struct rftype *rft = of->kn->priv;
> > > bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
> >
> > ok, I'll post the change
>
> v2 attached
>
> thanks,
> jirka
>
> ---
> While playing with the resctrl interface I found it much
> easier to deal with cpumask list rather than just regular
> cpumask.
>
> Adding cpus_list file to provide cpumask list interface
> to define group's cpus.
>
> # cd /sys/fs/resctrl/
> # echo 1-10 > krava/cpus_list
> # cat krava/cpus_list
> 1-10
> # cat krava/cpus
> 0007fe
> # cat cpus
> fffff9
> # cat cpus_list
> 0,3-23
Could you please have a right commit message and patch format?
git am can not pick up your commit message as you expected.
>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> arch/x86/include/asm/intel_rdt.h | 4 ++++
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
Could you please add cpus_list info in intel_rdt_ui.txt?
> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
> index 3f313991c0b3..a1a6681738a5 100644
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -37,6 +37,9 @@ struct rdtgroup {
> /* rdtgroup.flags */
> #define RDT_DELETED 1
>
> +/* rftype.flags */
> +#define RFTYPE_FLAGS_CPUS_LIST 1
> +
> /* List of all resource groups */
> extern struct list_head rdt_all_groups;
>
> @@ -56,6 +59,7 @@ struct rftype {
> char *name;
> umode_t mode;
> struct kernfs_ops *kf_ops;
> + unsigned long flags;
>
> int (*seq_show)(struct kernfs_open_file *of,
> struct seq_file *sf, void *v);
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 9ac2a5cdd9c2..b089d2452f1e 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
> .seq_show = rdtgroup_seqfile_show,
> };
>
> +static bool is_list(struct kernfs_open_file *of)
> +{
> + struct rftype *rft = of->kn->priv;
> +
> + return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
> +}
> +
> static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> struct seq_file *s, void *v)
> {
> @@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>
> rdtgrp = rdtgroup_kn_lock_live(of->kn);
>
> - if (rdtgrp)
> - seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> - else
> + if (rdtgrp) {
> + seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> + cpumask_pr_args(&rdtgrp->cpu_mask));
> + } else
Unbalanced braces around if-else. You can remove the { and } after if.
> ret = -ENOENT;
> rdtgroup_kn_unlock(of->kn);
>
> @@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
> goto unlock;
> }
>
> - ret = cpumask_parse(buf, newmask);
> + if (is_list(of))
> + ret = cpulist_parse(buf, newmask);
> + else
> + ret = cpumask_parse(buf, newmask);
> +
> if (ret)
> goto unlock;
>
> @@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
> .seq_show = rdtgroup_cpus_show,
> },
> {
> + .name = "cpus_list",
> + .mode = 0644,
> + .kf_ops = &rdtgroup_kf_single_ops,
> + .write = rdtgroup_cpus_write,
> + .seq_show = rdtgroup_cpus_show,
> + .flags = RFTYPE_FLAGS_CPUS_LIST,
> + },
> + {
> .name = "tasks",
> .mode = 0644,
> .kf_ops = &rdtgroup_kf_single_ops,
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-09 17:38 ` Fenghua Yu
@ 2017-04-10 7:40 ` Jiri Olsa
2017-04-10 9:45 ` Thomas Gleixner
2017-04-10 9:14 ` [PATCHv2] " Thomas Gleixner
1 sibling, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-04-10 7:40 UTC (permalink / raw)
To: Fenghua Yu
Cc: Thomas Gleixner, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.
Adding cpus_list file to provide cpumask list interface
to define group's cpus.
# cd /sys/fs/resctrl/
# echo 1-10 > krava/cpus_list
# cat krava/cpus_list
1-10
# cat krava/cpus
0007fe
# cat cpus
fffff9
# cat cpus_list
0,3-23
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/include/asm/intel_rdt.h | 4 ++++
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..a1a6681738a5 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
/* rdtgroup.flags */
#define RDT_DELETED 1
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST 1
+
/* List of all resource groups */
extern struct list_head rdt_all_groups;
@@ -56,6 +59,7 @@ struct rftype {
char *name;
umode_t mode;
struct kernfs_ops *kf_ops;
+ unsigned long flags;
int (*seq_show)(struct kernfs_open_file *of,
struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..b089d2452f1e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
.seq_show = rdtgroup_seqfile_show,
};
+static bool is_list(struct kernfs_open_file *of)
+{
+ struct rftype *rft = of->kn->priv;
+
+ return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
static int rdtgroup_cpus_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
rdtgrp = rdtgroup_kn_lock_live(of->kn);
- if (rdtgrp)
- seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
- else
+ if (rdtgrp) {
+ seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+ cpumask_pr_args(&rdtgrp->cpu_mask));
+ } else
ret = -ENOENT;
rdtgroup_kn_unlock(of->kn);
@@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = cpumask_parse(buf, newmask);
+ if (is_list(of))
+ ret = cpulist_parse(buf, newmask);
+ else
+ ret = cpumask_parse(buf, newmask);
+
if (ret)
goto unlock;
@@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
.seq_show = rdtgroup_cpus_show,
},
{
+ .name = "cpus_list",
+ .mode = 0644,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .write = rdtgroup_cpus_write,
+ .seq_show = rdtgroup_cpus_show,
+ .flags = RFTYPE_FLAGS_CPUS_LIST,
+ },
+ {
.name = "tasks",
.mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-09 17:38 ` Fenghua Yu
2017-04-10 7:40 ` [PATCHv3] " Jiri Olsa
@ 2017-04-10 9:14 ` Thomas Gleixner
1 sibling, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2017-04-10 9:14 UTC (permalink / raw)
To: Fenghua Yu
Cc: Jiri Olsa, Jiri Olsa, Peter Zijlstra, Mike Galbraith, Shaohua Li,
lkml, Ingo Molnar, Peter Zijlstra
On Sun, 9 Apr 2017, Fenghua Yu wrote:
> > rdtgrp = rdtgroup_kn_lock_live(of->kn);
> >
> > - if (rdtgrp)
> > - seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> > - else
> > + if (rdtgrp) {
> > + seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> > + cpumask_pr_args(&rdtgrp->cpu_mask));
> > + } else
>
> Unbalanced braces around if-else. You can remove the { and } after if.
Wrong. I've explained that a gazillion of times already.
http://lkml.kernel.org/r/alpine.DEB.2.20.1701171956290.3645@nanos
So yes, the else path wants braces.
Thanks,
tglx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-10 7:40 ` [PATCHv3] " Jiri Olsa
@ 2017-04-10 9:45 ` Thomas Gleixner
2017-04-10 14:52 ` [PATCHv4] " Jiri Olsa
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2017-04-10 9:45 UTC (permalink / raw)
To: Jiri Olsa
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
On Mon, 10 Apr 2017, Jiri Olsa wrote:
> /* List of all resource groups */
> extern struct list_head rdt_all_groups;
>
> @@ -56,6 +59,7 @@ struct rftype {
> char *name;
> umode_t mode;
> struct kernfs_ops *kf_ops;
> + unsigned long flags;
Lacks update in the kernel-doc comment.
> @@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>
> rdtgrp = rdtgroup_kn_lock_live(of->kn);
>
> - if (rdtgrp)
> - seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> - else
> + if (rdtgrp) {
> + seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> + cpumask_pr_args(&rdtgrp->cpu_mask));
> + } else
> ret = -ENOENT;
This lacks braces around the else path
> @@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
> .seq_show = rdtgroup_cpus_show,
> },
> {
> + .name = "cpus_list",
> + .mode = 0644,
> + .kf_ops = &rdtgroup_kf_single_ops,
> + .write = rdtgroup_cpus_write,
> + .seq_show = rdtgroup_cpus_show,
> + .flags = RFTYPE_FLAGS_CPUS_LIST,
> + },
Please add the new file to Documentation/x86/intel_rdt_ui.txt
Thanks,
tglx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv4] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-10 9:45 ` Thomas Gleixner
@ 2017-04-10 14:52 ` Jiri Olsa
2017-04-10 17:16 ` [tip:x86/cpu] " tip-bot for Jiri Olsa
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2017-04-10 14:52 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fenghua Yu, Jiri Olsa, Peter Zijlstra, Mike Galbraith,
Shaohua Li, lkml, Ingo Molnar, Peter Zijlstra
While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.
Adding cpus_list file to provide cpumask list interface
to define group's cpus.
# cd /sys/fs/resctrl/
# echo 1-10 > krava/cpus_list
# cat krava/cpus_list
1-10
# cat krava/cpus
0007fe
# cat cpus
fffff9
# cat cpus_list
0,3-23
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
Documentation/x86/intel_rdt_ui.txt | 3 +++
arch/x86/include/asm/intel_rdt.h | 5 +++++
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 29 +++++++++++++++++++++++++----
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/Documentation/x86/intel_rdt_ui.txt b/Documentation/x86/intel_rdt_ui.txt
index 3ea198460469..7029071e2373 100644
--- a/Documentation/x86/intel_rdt_ui.txt
+++ b/Documentation/x86/intel_rdt_ui.txt
@@ -59,6 +59,9 @@ There are three files associated with each group:
given to the default (root) group. You cannot remove CPUs
from the default group.
+"cpus_list": A bitmask *list* of logical CPUs assigned to this group.
+ Same rules apply like for the "cpus" file.
+
"schemata": A list of all the resources available to this group.
Each resource has its own line and format - see below for
details.
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..9069402f7497 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
/* rdtgroup.flags */
#define RDT_DELETED 1
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST 1
+
/* List of all resource groups */
extern struct list_head rdt_all_groups;
@@ -49,6 +52,7 @@ struct rdtgroup {
* @name: file name
* @mode: access mode
* @kf_ops: operations
+ * flags: RFTYPE_FLAGS_* flags
* @seq_show: show content of the file
* @write: write to the file
*/
@@ -56,6 +60,7 @@ struct rftype {
char *name;
umode_t mode;
struct kernfs_ops *kf_ops;
+ unsigned long flags;
int (*seq_show)(struct kernfs_open_file *of,
struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..56b5bdaf98e1 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
.seq_show = rdtgroup_seqfile_show,
};
+static bool is_list(struct kernfs_open_file *of)
+{
+ struct rftype *rft = of->kn->priv;
+
+ return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
static int rdtgroup_cpus_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -182,10 +189,12 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
rdtgrp = rdtgroup_kn_lock_live(of->kn);
- if (rdtgrp)
- seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
- else
+ if (rdtgrp) {
+ seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+ cpumask_pr_args(&rdtgrp->cpu_mask));
+ } else {
ret = -ENOENT;
+ }
rdtgroup_kn_unlock(of->kn);
return ret;
@@ -252,7 +261,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = cpumask_parse(buf, newmask);
+ if (is_list(of))
+ ret = cpulist_parse(buf, newmask);
+ else
+ ret = cpumask_parse(buf, newmask);
+
if (ret)
goto unlock;
@@ -473,6 +486,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
.seq_show = rdtgroup_cpus_show,
},
{
+ .name = "cpus_list",
+ .mode = 0644,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .write = rdtgroup_cpus_write,
+ .seq_show = rdtgroup_cpus_show,
+ .flags = RFTYPE_FLAGS_CPUS_LIST,
+ },
+ {
.name = "tasks",
.mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/cpu] x86/intel_rdt: Add cpus_list rdtgroup file
2017-04-10 14:52 ` [PATCHv4] " Jiri Olsa
@ 2017-04-10 17:16 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-04-10 17:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, peterz, jolsa, tglx, shli, efault, mingo, a.p.zijlstra,
linux-kernel, fenghua.yu, hpa
Commit-ID: 4ffa3c977b5da2907eb294dc6d0259a02f2284aa
Gitweb: http://git.kernel.org/tip/4ffa3c977b5da2907eb294dc6d0259a02f2284aa
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 10 Apr 2017 16:52:32 +0200
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 10 Apr 2017 19:10:25 +0200
x86/intel_rdt: Add cpus_list rdtgroup file
The resource control filesystem provides only a bitmask based cpus file for
assigning CPUs to a resource group. That's cumbersome with large cpumasks
and non-intuitive when modifying the file from the command line.
Range based cpu lists are commonly used along with bitmask based cpu files
in various subsystems throughout the kernel.
Add 'cpus_list' file which is CPU range based.
# cd /sys/fs/resctrl/
# echo 1-10 > krava/cpus_list
# cat krava/cpus_list
1-10
# cat krava/cpus
0007fe
# cat cpus
fffff9
# cat cpus_list
0,3-23
[ tglx: Massaged changelog and replaced "bitmask lists" by "CPU ranges" ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Link: http://lkml.kernel.org/r/20170410145232.GF25354@krava
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
Documentation/x86/intel_rdt_ui.txt | 3 +++
arch/x86/include/asm/intel_rdt.h | 5 +++++
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 29 +++++++++++++++++++++++++----
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/Documentation/x86/intel_rdt_ui.txt b/Documentation/x86/intel_rdt_ui.txt
index 3ea1984..a1ace91 100644
--- a/Documentation/x86/intel_rdt_ui.txt
+++ b/Documentation/x86/intel_rdt_ui.txt
@@ -59,6 +59,9 @@ There are three files associated with each group:
given to the default (root) group. You cannot remove CPUs
from the default group.
+"cpus_list": One or more CPU ranges of logical CPUs assigned to this
+ group. Same rules apply like for the "cpus" file.
+
"schemata": A list of all the resources available to this group.
Each resource has its own line and format - see below for
details.
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 9030bcc..611c823 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
/* rdtgroup.flags */
#define RDT_DELETED 1
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST 1
+
/* List of all resource groups */
extern struct list_head rdt_all_groups;
@@ -49,6 +52,7 @@ int __init rdtgroup_init(void);
* @name: File name
* @mode: Access mode
* @kf_ops: File operations
+ * @flags: File specific RFTYPE_FLAGS_* flags
* @seq_show: Show content of the file
* @write: Write to the file
*/
@@ -56,6 +60,7 @@ struct rftype {
char *name;
umode_t mode;
struct kernfs_ops *kf_ops;
+ unsigned long flags;
int (*seq_show)(struct kernfs_open_file *of,
struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index c05509d..8401cf5 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static struct kernfs_ops rdtgroup_kf_single_ops = {
.seq_show = rdtgroup_seqfile_show,
};
+static bool is_cpu_list(struct kernfs_open_file *of)
+{
+ struct rftype *rft = of->kn->priv;
+
+ return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
static int rdtgroup_cpus_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -182,10 +189,12 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
rdtgrp = rdtgroup_kn_lock_live(of->kn);
- if (rdtgrp)
- seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
- else
+ if (rdtgrp) {
+ seq_printf(s, is_cpu_list(of) ? "%*pbl\n" : "%*pb\n",
+ cpumask_pr_args(&rdtgrp->cpu_mask));
+ } else {
ret = -ENOENT;
+ }
rdtgroup_kn_unlock(of->kn);
return ret;
@@ -252,7 +261,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = cpumask_parse(buf, newmask);
+ if (is_cpu_list(of))
+ ret = cpulist_parse(buf, newmask);
+ else
+ ret = cpumask_parse(buf, newmask);
+
if (ret)
goto unlock;
@@ -473,6 +486,14 @@ static struct rftype rdtgroup_base_files[] = {
.seq_show = rdtgroup_cpus_show,
},
{
+ .name = "cpus_list",
+ .mode = 0644,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .write = rdtgroup_cpus_write,
+ .seq_show = rdtgroup_cpus_show,
+ .flags = RFTYPE_FLAGS_CPUS_LIST,
+ },
+ {
.name = "tasks",
.mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-04-10 17:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-29 15:09 [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa
2017-03-29 16:08 ` Fenghua Yu
2017-03-29 16:16 ` Jiri Olsa
2017-03-30 14:59 ` Jiri Olsa
2017-04-01 2:00 ` Fenghua Yu
2017-03-31 8:47 ` Thomas Gleixner
2017-03-31 15:53 ` Fenghua Yu
2017-03-31 9:15 ` Thomas Gleixner
2017-03-31 15:40 ` Fenghua Yu
2017-03-31 15:55 ` Jiri Olsa
2017-04-09 16:49 ` [PATCHv2] " Jiri Olsa
2017-04-09 17:38 ` Fenghua Yu
2017-04-10 7:40 ` [PATCHv3] " Jiri Olsa
2017-04-10 9:45 ` Thomas Gleixner
2017-04-10 14:52 ` [PATCHv4] " Jiri Olsa
2017-04-10 17:16 ` [tip:x86/cpu] " tip-bot for Jiri Olsa
2017-04-10 9:14 ` [PATCHv2] " Thomas Gleixner
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).