Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Wed, 1 Mar 2017, Thomas Gleixner wrote:
On Fri, 17 Feb 2017, Vikas Shivappa wrote:
Add files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- min_bw: the minimum memory bandwidth(b/w) values in percentage.
OS maps the b/w percentage values to memory b/w throttle delay values
and configures them via MSR interface by writing to the QOS_MSRs. These
delay values can have a linear or nonlinear scale.
- bw_gran: The memory b/w granularity that can be configured.
For ex: If the granularity is 10% and min_bw is 10, valid bandwidth
values are 10,20,30...
This is unreadable. It's possible to structure ASCII text cleanly.
x86/rdt: Add info directory files for MBA
The info directory for MBA contains the following files:
num_closids
The maximum number of class of service slots available for MBA
min_bandwidth
The minimum memory bandwidth percentage value
bandwidth_gran
The granularity of the bandwidth control for the particular
hardware in percent. The available bandwidth control steps are:
min_bw + N * bw_gran Intermediate values are rounded to the next
Is this next or pervious? Meaning when 12 is requested on a 10 granularity , we
give 10 ?
control step available on the hardware.
delay_linear
If set, the control registers take a linear percentage based value
between min_bandwidth and 100 percent.
If not set, the control registers take a power of 2 based value
which is mapped by the kernel to percentage based values.
This file is of pure informational nature and has no influence on
the values which are written to the schemata files. These are
always percentage based.
Will update the changelogs
Note, that this uses the actual file names and not some random
abbreviations thereof. It also documents delay_linear and gets rid of the
implementation details of QOS_MSRs. They are irrelevant here.
And exactly this information wants to go into Documentation/... preferably
in exactly this patch and not in a disconnected one which describes stuff
differently for whatever reasons.
+static int rdt_min_bw_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n", r->min_bw);
+
Can you please get rid of these pointless extra new lines before the
'return 0;' ? They are just eating screen estate and do not make the code
more readable.
All the rest of the show functions have that line before return like the
rdt_min_cbm_bits_show etc. I have tried to
always keep a line before return like the old cqm/rapl - if thats ok
+/* rdtgroup information files for MBE. */
What is MBE?
+static struct rftype res_mbe_info_files[] = {
Randomizing names make the code more secure or what are you trying to
achieve?
+void rdt_get_mba_infofile(struct rdt_resource *r)
+{
+ r->info_files = _mbe_info_files[0];
See other mail.
Will fix the MBE to MBA and the pointer init.
Thanks,
Vikas
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Wed, 1 Mar 2017, Thomas Gleixner wrote:
On Fri, 17 Feb 2017, Vikas Shivappa wrote:
Add files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- min_bw: the minimum memory bandwidth(b/w) values in percentage.
OS maps the b/w percentage values to memory b/w throttle delay values
and configures them via MSR interface by writing to the QOS_MSRs. These
delay values can have a linear or nonlinear scale.
- bw_gran: The memory b/w granularity that can be configured.
For ex: If the granularity is 10% and min_bw is 10, valid bandwidth
values are 10,20,30...
This is unreadable. It's possible to structure ASCII text cleanly.
x86/rdt: Add info directory files for MBA
The info directory for MBA contains the following files:
num_closids
The maximum number of class of service slots available for MBA
min_bandwidth
The minimum memory bandwidth percentage value
bandwidth_gran
The granularity of the bandwidth control for the particular
hardware in percent. The available bandwidth control steps are:
min_bw + N * bw_gran Intermediate values are rounded to the next
Is this next or pervious? Meaning when 12 is requested on a 10 granularity , we
give 10 ?
control step available on the hardware.
delay_linear
If set, the control registers take a linear percentage based value
between min_bandwidth and 100 percent.
If not set, the control registers take a power of 2 based value
which is mapped by the kernel to percentage based values.
This file is of pure informational nature and has no influence on
the values which are written to the schemata files. These are
always percentage based.
Will update the changelogs
Note, that this uses the actual file names and not some random
abbreviations thereof. It also documents delay_linear and gets rid of the
implementation details of QOS_MSRs. They are irrelevant here.
And exactly this information wants to go into Documentation/... preferably
in exactly this patch and not in a disconnected one which describes stuff
differently for whatever reasons.
+static int rdt_min_bw_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n", r->min_bw);
+
Can you please get rid of these pointless extra new lines before the
'return 0;' ? They are just eating screen estate and do not make the code
more readable.
All the rest of the show functions have that line before return like the
rdt_min_cbm_bits_show etc. I have tried to
always keep a line before return like the old cqm/rapl - if thats ok
+/* rdtgroup information files for MBE. */
What is MBE?
+static struct rftype res_mbe_info_files[] = {
Randomizing names make the code more secure or what are you trying to
achieve?
+void rdt_get_mba_infofile(struct rdt_resource *r)
+{
+ r->info_files = _mbe_info_files[0];
See other mail.
Will fix the MBE to MBA and the pointer init.
Thanks,
Vikas
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Fri, 17 Feb 2017, Vikas Shivappa wrote:
> Add files in info directory for MBA.
> The files in the info directory are as follows :
> - num_closids: max number of closids for MBA which represents the max
> class of service user can configure.
> - min_bw: the minimum memory bandwidth(b/w) values in percentage.
>
> OS maps the b/w percentage values to memory b/w throttle delay values
> and configures them via MSR interface by writing to the QOS_MSRs. These
> delay values can have a linear or nonlinear scale.
>
> - bw_gran: The memory b/w granularity that can be configured.
> For ex: If the granularity is 10% and min_bw is 10, valid bandwidth
> values are 10,20,30...
This is unreadable. It's possible to structure ASCII text cleanly.
x86/rdt: Add info directory files for MBA
The info directory for MBA contains the following files:
num_closids
The maximum number of class of service slots available for MBA
min_bandwidth
The minimum memory bandwidth percentage value
bandwidth_gran
The granularity of the bandwidth control for the particular
hardware in percent. The available bandwidth control steps are:
min_bw + N * bw_gran Intermediate values are rounded to the next
control step available on the hardware.
delay_linear
If set, the control registers take a linear percentage based value
between min_bandwidth and 100 percent.
If not set, the control registers take a power of 2 based value
which is mapped by the kernel to percentage based values.
This file is of pure informational nature and has no influence on
the values which are written to the schemata files. These are
always percentage based.
Note, that this uses the actual file names and not some random
abbreviations thereof. It also documents delay_linear and gets rid of the
implementation details of QOS_MSRs. They are irrelevant here.
And exactly this information wants to go into Documentation/... preferably
in exactly this patch and not in a disconnected one which describes stuff
differently for whatever reasons.
> +static int rdt_min_bw_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->min_bw);
> +
Can you please get rid of these pointless extra new lines before the
'return 0;' ? They are just eating screen estate and do not make the code
more readable.
> +/* rdtgroup information files for MBE. */
What is MBE?
> +static struct rftype res_mbe_info_files[] = {
Randomizing names make the code more secure or what are you trying to
achieve?
> +void rdt_get_mba_infofile(struct rdt_resource *r)
> +{
> + r->info_files = _mbe_info_files[0];
See other mail.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Fri, 17 Feb 2017, Vikas Shivappa wrote:
> Add files in info directory for MBA.
> The files in the info directory are as follows :
> - num_closids: max number of closids for MBA which represents the max
> class of service user can configure.
> - min_bw: the minimum memory bandwidth(b/w) values in percentage.
>
> OS maps the b/w percentage values to memory b/w throttle delay values
> and configures them via MSR interface by writing to the QOS_MSRs. These
> delay values can have a linear or nonlinear scale.
>
> - bw_gran: The memory b/w granularity that can be configured.
> For ex: If the granularity is 10% and min_bw is 10, valid bandwidth
> values are 10,20,30...
This is unreadable. It's possible to structure ASCII text cleanly.
x86/rdt: Add info directory files for MBA
The info directory for MBA contains the following files:
num_closids
The maximum number of class of service slots available for MBA
min_bandwidth
The minimum memory bandwidth percentage value
bandwidth_gran
The granularity of the bandwidth control for the particular
hardware in percent. The available bandwidth control steps are:
min_bw + N * bw_gran Intermediate values are rounded to the next
control step available on the hardware.
delay_linear
If set, the control registers take a linear percentage based value
between min_bandwidth and 100 percent.
If not set, the control registers take a power of 2 based value
which is mapped by the kernel to percentage based values.
This file is of pure informational nature and has no influence on
the values which are written to the schemata files. These are
always percentage based.
Note, that this uses the actual file names and not some random
abbreviations thereof. It also documents delay_linear and gets rid of the
implementation details of QOS_MSRs. They are irrelevant here.
And exactly this information wants to go into Documentation/... preferably
in exactly this patch and not in a disconnected one which describes stuff
differently for whatever reasons.
> +static int rdt_min_bw_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->min_bw);
> +
Can you please get rid of these pointless extra new lines before the
'return 0;' ? They are just eating screen estate and do not make the code
more readable.
> +/* rdtgroup information files for MBE. */
What is MBE?
> +static struct rftype res_mbe_info_files[] = {
Randomizing names make the code more secure or what are you trying to
achieve?
> +void rdt_get_mba_infofile(struct rdt_resource *r)
> +{
> + r->info_files = _mbe_info_files[0];
See other mail.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Wed, 18 Jan 2017, Thomas Gleixner wrote:
On Tue, 17 Jan 2017, Shivappa Vikas wrote:
On Mon, 16 Jan 2017, Thomas Gleixner wrote:
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
Add the files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- max_thrtl_by: the max throttle by values.
Throttle by can have a linear scale and non linear scale. In linear
scale, if a throttle_by value is 40%, it means that the memory b/w is
throttled 'by' 40% or in other words a max of 60% b/w is allowed to
pass. In non-linear scale, the throttle_by values are in 2^n
granularity. The h/w does not guarantee a curve of actual throttle w.r.t
the configured values. But if a throttle_by value of x > y, then x is
guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
The x and y are actual values written which i will spell out. The assumption
is that only correct values are input though because we filterout the values
which dont follow granularity, meaning return -EINVAL when someone tries to
write 11 when granularity is 10. This was with the idea thats its easier for
the user to understand whats actually written. Woudl that be reasonable or
does it need a change ?
(Although the h/w does like you say , we can do a msr write for 11 etc ..)
+/* rdtgroup information files for MBE. */
+static struct rftype res_mbe_info_files[] = {
+ {
+ .name = "num_closids",
+ .mode = 0444,
+ .kf_ops = _kf_single_ops,
+ .seq_show = rdt_num_closids_show,
+ },
+ {
+ .name = "max_thrtl_by",
You surely could not come up with a more cryptic file name, right? What's
so wrong with spelling out throttle? And the whole '_by' postfix here and
on the other files is pointless as well.
This is due to the issue i mention in reply to 1/8.. Can be changed to
Err no. max_thrtl_by has nothing to do with 1/8. Of course you want to
expose the hardware information.
What's wrong with spelling out throttle and remove the '_by':
max_throttle
Always was under the impression max_throttle by default means the max b/w thats
allowed. (if max_throttle is 90, user would think 90% is whats the most allowed
to pass..). But this is really the delay value which was implemented which
means the % of bytes that are 'not' allowed to pass (so 90 means , that 90% is whats
restricted and 10 is what is allowed).
Thanks,
Vikas
That is readable, understandable and matches the documentation in the SDM.
It's not that hard.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Wed, 18 Jan 2017, Thomas Gleixner wrote:
On Tue, 17 Jan 2017, Shivappa Vikas wrote:
On Mon, 16 Jan 2017, Thomas Gleixner wrote:
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
Add the files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- max_thrtl_by: the max throttle by values.
Throttle by can have a linear scale and non linear scale. In linear
scale, if a throttle_by value is 40%, it means that the memory b/w is
throttled 'by' 40% or in other words a max of 60% b/w is allowed to
pass. In non-linear scale, the throttle_by values are in 2^n
granularity. The h/w does not guarantee a curve of actual throttle w.r.t
the configured values. But if a throttle_by value of x > y, then x is
guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
The x and y are actual values written which i will spell out. The assumption
is that only correct values are input though because we filterout the values
which dont follow granularity, meaning return -EINVAL when someone tries to
write 11 when granularity is 10. This was with the idea thats its easier for
the user to understand whats actually written. Woudl that be reasonable or
does it need a change ?
(Although the h/w does like you say , we can do a msr write for 11 etc ..)
+/* rdtgroup information files for MBE. */
+static struct rftype res_mbe_info_files[] = {
+ {
+ .name = "num_closids",
+ .mode = 0444,
+ .kf_ops = _kf_single_ops,
+ .seq_show = rdt_num_closids_show,
+ },
+ {
+ .name = "max_thrtl_by",
You surely could not come up with a more cryptic file name, right? What's
so wrong with spelling out throttle? And the whole '_by' postfix here and
on the other files is pointless as well.
This is due to the issue i mention in reply to 1/8.. Can be changed to
Err no. max_thrtl_by has nothing to do with 1/8. Of course you want to
expose the hardware information.
What's wrong with spelling out throttle and remove the '_by':
max_throttle
Always was under the impression max_throttle by default means the max b/w thats
allowed. (if max_throttle is 90, user would think 90% is whats the most allowed
to pass..). But this is really the delay value which was implemented which
means the % of bytes that are 'not' allowed to pass (so 90 means , that 90% is whats
restricted and 10 is what is allowed).
Thanks,
Vikas
That is readable, understandable and matches the documentation in the SDM.
It's not that hard.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Tue, 17 Jan 2017, Shivappa Vikas wrote:
> On Mon, 16 Jan 2017, Thomas Gleixner wrote:
>
> > On Tue, 10 Jan 2017, Vikas Shivappa wrote:
> >
> > > Add the files in info directory for MBA.
> > > The files in the info directory are as follows :
> > > - num_closids: max number of closids for MBA which represents the max
> > > class of service user can configure.
> > > - max_thrtl_by: the max throttle by values.
> > >
> > > Throttle by can have a linear scale and non linear scale. In linear
> > > scale, if a throttle_by value is 40%, it means that the memory b/w is
> > > throttled 'by' 40% or in other words a max of 60% b/w is allowed to
> > > pass. In non-linear scale, the throttle_by values are in 2^n
> > > granularity. The h/w does not guarantee a curve of actual throttle w.r.t
> > > the configured values. But if a throttle_by value of x > y, then x is
> > > guaranteed to throttle more b/w than y.
> >
> > This is ambigous because that is only correct when the effective values are
> > different. x=11 and y=12 with a granularity of 10 are resulting in the same
> > throttling.
>
> The x and y are actual values written which i will spell out. The assumption
> is that only correct values are input though because we filterout the values
> which dont follow granularity, meaning return -EINVAL when someone tries to
> write 11 when granularity is 10. This was with the idea thats its easier for
> the user to understand whats actually written. Woudl that be reasonable or
> does it need a change ?
> (Although the h/w does like you say , we can do a msr write for 11 etc ..)
> > > +/* rdtgroup information files for MBE. */
> > > +static struct rftype res_mbe_info_files[] = {
> > > + {
> > > + .name = "num_closids",
> > > + .mode = 0444,
> > > + .kf_ops = _kf_single_ops,
> > > + .seq_show = rdt_num_closids_show,
> > > + },
> > > + {
> > > + .name = "max_thrtl_by",
> >
> > You surely could not come up with a more cryptic file name, right? What's
> > so wrong with spelling out throttle? And the whole '_by' postfix here and
> > on the other files is pointless as well.
>
> This is due to the issue i mention in reply to 1/8.. Can be changed to
Err no. max_thrtl_by has nothing to do with 1/8. Of course you want to
expose the hardware information.
What's wrong with spelling out throttle and remove the '_by':
max_throttle
That is readable, understandable and matches the documentation in the SDM.
It's not that hard.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Tue, 17 Jan 2017, Shivappa Vikas wrote:
> On Mon, 16 Jan 2017, Thomas Gleixner wrote:
>
> > On Tue, 10 Jan 2017, Vikas Shivappa wrote:
> >
> > > Add the files in info directory for MBA.
> > > The files in the info directory are as follows :
> > > - num_closids: max number of closids for MBA which represents the max
> > > class of service user can configure.
> > > - max_thrtl_by: the max throttle by values.
> > >
> > > Throttle by can have a linear scale and non linear scale. In linear
> > > scale, if a throttle_by value is 40%, it means that the memory b/w is
> > > throttled 'by' 40% or in other words a max of 60% b/w is allowed to
> > > pass. In non-linear scale, the throttle_by values are in 2^n
> > > granularity. The h/w does not guarantee a curve of actual throttle w.r.t
> > > the configured values. But if a throttle_by value of x > y, then x is
> > > guaranteed to throttle more b/w than y.
> >
> > This is ambigous because that is only correct when the effective values are
> > different. x=11 and y=12 with a granularity of 10 are resulting in the same
> > throttling.
>
> The x and y are actual values written which i will spell out. The assumption
> is that only correct values are input though because we filterout the values
> which dont follow granularity, meaning return -EINVAL when someone tries to
> write 11 when granularity is 10. This was with the idea thats its easier for
> the user to understand whats actually written. Woudl that be reasonable or
> does it need a change ?
> (Although the h/w does like you say , we can do a msr write for 11 etc ..)
> > > +/* rdtgroup information files for MBE. */
> > > +static struct rftype res_mbe_info_files[] = {
> > > + {
> > > + .name = "num_closids",
> > > + .mode = 0444,
> > > + .kf_ops = _kf_single_ops,
> > > + .seq_show = rdt_num_closids_show,
> > > + },
> > > + {
> > > + .name = "max_thrtl_by",
> >
> > You surely could not come up with a more cryptic file name, right? What's
> > so wrong with spelling out throttle? And the whole '_by' postfix here and
> > on the other files is pointless as well.
>
> This is due to the issue i mention in reply to 1/8.. Can be changed to
Err no. max_thrtl_by has nothing to do with 1/8. Of course you want to
expose the hardware information.
What's wrong with spelling out throttle and remove the '_by':
max_throttle
That is readable, understandable and matches the documentation in the SDM.
It's not that hard.
Thanks,
tglx
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Mon, 16 Jan 2017, Thomas Gleixner wrote:
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
Add the files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- max_thrtl_by: the max throttle by values.
Throttle by can have a linear scale and non linear scale. In linear
scale, if a throttle_by value is 40%, it means that the memory b/w is
throttled 'by' 40% or in other words a max of 60% b/w is allowed to
pass. In non-linear scale, the throttle_by values are in 2^n
granularity. The h/w does not guarantee a curve of actual throttle w.r.t
the configured values. But if a throttle_by value of x > y, then x is
guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
The x and y are actual values written which i will spell out. The assumption is
that only correct values are input though because we filterout the values which
dont follow granularity, meaning return -EINVAL
when someone tries to write 11 when granularity is 10. This was with the idea
thats its easier for the user to understand whats actually written. Woudl that
be reasonable or does it need a change ?
(Although the h/w does like you say , we can do a msr write for 11 etc ..)
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -5,7 +5,6 @@
#include
#include
-
#include
This white space is there on purpose to seperate linux and asm prefixed
includes. Stop making random white space changes.
@@ -77,6 +76,8 @@ struct rftype {
* @no_ctrl: Specifies max cache cbm or min mem b/w delay.
* @min_cbm_bits: Minimum number of consecutive bits to be set
* in a cache bit mask
+ * @info_files:resctrl info files for the resource
+ * @infofiles_len: Number of info files
This wants to be a seperate patch as it changes the way how the info files
are set up. The implementation for the MBA stuff is a follow up patch.
Ok , will split
* @max_delay: Max throttle delay
* @delay_gran:Throttle delay granularity
* @delay_linear: true if delay is in linear scale
@@ -98,6 +99,8 @@ struct rdt_resource {
int cbm_len;
int min_cbm_bits;
u32 no_ctrl;
+ struct rftype *info_files;
+ int infofiles_len;
u32 max_delay;
u32 delay_gran;
u32 delay_linear;
@@ -137,6 +140,9 @@ struct msr_param {
int high;
};
+void rdt_get_cache_infofile(struct rdt_resource *r);
+void rdt_get_mbe_infofile(struct rdt_resource *r);
+
extern struct mutex rdtgroup_mutex;
extern struct rdt_resource rdt_resources_all[];
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 6736e1d..bdfbd1d 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -154,6 +154,7 @@ static void rdt_get_mem_config(struct rdt_resource *r)
if (r->delay_linear)
r->delay_gran = MAX_MBA_THRTL - r->max_delay;
+ rdt_get_mbe_infofile(r);
r->capable = true;
r->enabled = true;
}
@@ -168,6 +169,7 @@ static void rdt_get_cache_config(int idx, struct
rdt_resource *r)
r->num_closid = edx.split.cos_max + 1;
r->cbm_len = eax.split.cbm_len + 1;
r->no_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
+ rdt_get_cache_infofile(r);
r->capable = true;
r->enabled = true;
}
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 53f1917..9d9b7f4 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -517,9 +517,41 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file
*of,
return 0;
}
+static int rdt_max_delay_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n", r->max_delay);
+
+ return 0;
+}
+
+static int rdt_delay_gran_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ if (r->delay_linear)
+ seq_printf(seq, "%d\n", r->delay_gran);
+ else
+ seq_printf(seq, "power of 2\n");
+
+ return 0;
+}
+
+static int rdt_delay_linear_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n",
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Mon, 16 Jan 2017, Thomas Gleixner wrote:
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
Add the files in info directory for MBA.
The files in the info directory are as follows :
- num_closids: max number of closids for MBA which represents the max
class of service user can configure.
- max_thrtl_by: the max throttle by values.
Throttle by can have a linear scale and non linear scale. In linear
scale, if a throttle_by value is 40%, it means that the memory b/w is
throttled 'by' 40% or in other words a max of 60% b/w is allowed to
pass. In non-linear scale, the throttle_by values are in 2^n
granularity. The h/w does not guarantee a curve of actual throttle w.r.t
the configured values. But if a throttle_by value of x > y, then x is
guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
The x and y are actual values written which i will spell out. The assumption is
that only correct values are input though because we filterout the values which
dont follow granularity, meaning return -EINVAL
when someone tries to write 11 when granularity is 10. This was with the idea
thats its easier for the user to understand whats actually written. Woudl that
be reasonable or does it need a change ?
(Although the h/w does like you say , we can do a msr write for 11 etc ..)
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -5,7 +5,6 @@
#include
#include
-
#include
This white space is there on purpose to seperate linux and asm prefixed
includes. Stop making random white space changes.
@@ -77,6 +76,8 @@ struct rftype {
* @no_ctrl: Specifies max cache cbm or min mem b/w delay.
* @min_cbm_bits: Minimum number of consecutive bits to be set
* in a cache bit mask
+ * @info_files:resctrl info files for the resource
+ * @infofiles_len: Number of info files
This wants to be a seperate patch as it changes the way how the info files
are set up. The implementation for the MBA stuff is a follow up patch.
Ok , will split
* @max_delay: Max throttle delay
* @delay_gran:Throttle delay granularity
* @delay_linear: true if delay is in linear scale
@@ -98,6 +99,8 @@ struct rdt_resource {
int cbm_len;
int min_cbm_bits;
u32 no_ctrl;
+ struct rftype *info_files;
+ int infofiles_len;
u32 max_delay;
u32 delay_gran;
u32 delay_linear;
@@ -137,6 +140,9 @@ struct msr_param {
int high;
};
+void rdt_get_cache_infofile(struct rdt_resource *r);
+void rdt_get_mbe_infofile(struct rdt_resource *r);
+
extern struct mutex rdtgroup_mutex;
extern struct rdt_resource rdt_resources_all[];
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 6736e1d..bdfbd1d 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -154,6 +154,7 @@ static void rdt_get_mem_config(struct rdt_resource *r)
if (r->delay_linear)
r->delay_gran = MAX_MBA_THRTL - r->max_delay;
+ rdt_get_mbe_infofile(r);
r->capable = true;
r->enabled = true;
}
@@ -168,6 +169,7 @@ static void rdt_get_cache_config(int idx, struct
rdt_resource *r)
r->num_closid = edx.split.cos_max + 1;
r->cbm_len = eax.split.cbm_len + 1;
r->no_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
+ rdt_get_cache_infofile(r);
r->capable = true;
r->enabled = true;
}
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 53f1917..9d9b7f4 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -517,9 +517,41 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file
*of,
return 0;
}
+static int rdt_max_delay_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n", r->max_delay);
+
+ return 0;
+}
+
+static int rdt_delay_gran_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ if (r->delay_linear)
+ seq_printf(seq, "%d\n", r->delay_gran);
+ else
+ seq_printf(seq, "power of 2\n");
+
+ return 0;
+}
+
+static int rdt_delay_linear_show(struct kernfs_open_file *of,
+struct seq_file *seq, void *v)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+
+ seq_printf(seq, "%d\n",
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
> Add the files in info directory for MBA.
> The files in the info directory are as follows :
> - num_closids: max number of closids for MBA which represents the max
> class of service user can configure.
> - max_thrtl_by: the max throttle by values.
>
> Throttle by can have a linear scale and non linear scale. In linear
> scale, if a throttle_by value is 40%, it means that the memory b/w is
> throttled 'by' 40% or in other words a max of 60% b/w is allowed to
> pass. In non-linear scale, the throttle_by values are in 2^n
> granularity. The h/w does not guarantee a curve of actual throttle w.r.t
> the configured values. But if a throttle_by value of x > y, then x is
> guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -5,7 +5,6 @@
>
> #include
> #include
> -
> #include
This white space is there on purpose to seperate linux and asm prefixed
includes. Stop making random white space changes.
> @@ -77,6 +76,8 @@ struct rftype {
> * @no_ctrl: Specifies max cache cbm or min mem b/w delay.
> * @min_cbm_bits:Minimum number of consecutive bits to be set
> * in a cache bit mask
> + * @info_files: resctrl info files for the resource
> + * @infofiles_len: Number of info files
This wants to be a seperate patch as it changes the way how the info files
are set up. The implementation for the MBA stuff is a follow up patch.
> * @max_delay: Max throttle delay
> * @delay_gran: Throttle delay granularity
> * @delay_linear:true if delay is in linear scale
> @@ -98,6 +99,8 @@ struct rdt_resource {
> int cbm_len;
> int min_cbm_bits;
> u32 no_ctrl;
> + struct rftype *info_files;
> + int infofiles_len;
> u32 max_delay;
> u32 delay_gran;
> u32 delay_linear;
> @@ -137,6 +140,9 @@ struct msr_param {
> int high;
> };
>
> +void rdt_get_cache_infofile(struct rdt_resource *r);
> +void rdt_get_mbe_infofile(struct rdt_resource *r);
> +
> extern struct mutex rdtgroup_mutex;
>
> extern struct rdt_resource rdt_resources_all[];
> diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
> index 6736e1d..bdfbd1d 100644
> --- a/arch/x86/kernel/cpu/intel_rdt.c
> +++ b/arch/x86/kernel/cpu/intel_rdt.c
> @@ -154,6 +154,7 @@ static void rdt_get_mem_config(struct rdt_resource *r)
> if (r->delay_linear)
> r->delay_gran = MAX_MBA_THRTL - r->max_delay;
>
> + rdt_get_mbe_infofile(r);
> r->capable = true;
> r->enabled = true;
> }
> @@ -168,6 +169,7 @@ static void rdt_get_cache_config(int idx, struct
> rdt_resource *r)
> r->num_closid = edx.split.cos_max + 1;
> r->cbm_len = eax.split.cbm_len + 1;
> r->no_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
> + rdt_get_cache_infofile(r);
> r->capable = true;
> r->enabled = true;
> }
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 53f1917..9d9b7f4 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -517,9 +517,41 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file
> *of,
>
> return 0;
> }
> +static int rdt_max_delay_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->max_delay);
> +
> + return 0;
> +}
> +
> +static int rdt_delay_gran_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + if (r->delay_linear)
> + seq_printf(seq, "%d\n", r->delay_gran);
> + else
> + seq_printf(seq, "power of 2\n");
> +
> + return 0;
> +}
> +
> +static int rdt_delay_linear_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->delay_linear);
> +
> + return 0;
> +}
>
> /* rdtgroup information files for one cache resource. */
> -static struct rftype res_info_files[] = {
> +static struct rftype res_cache_info_files[] = {
> {
> .name = "num_closids",
> .mode = 0444,
> @@ -540,11 +572,52 @@ static int rdt_min_cbm_bits_show(struct
> kernfs_open_file *of,
> },
> };
>
> +/* rdtgroup
Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA
On Tue, 10 Jan 2017, Vikas Shivappa wrote:
> Add the files in info directory for MBA.
> The files in the info directory are as follows :
> - num_closids: max number of closids for MBA which represents the max
> class of service user can configure.
> - max_thrtl_by: the max throttle by values.
>
> Throttle by can have a linear scale and non linear scale. In linear
> scale, if a throttle_by value is 40%, it means that the memory b/w is
> throttled 'by' 40% or in other words a max of 60% b/w is allowed to
> pass. In non-linear scale, the throttle_by values are in 2^n
> granularity. The h/w does not guarantee a curve of actual throttle w.r.t
> the configured values. But if a throttle_by value of x > y, then x is
> guaranteed to throttle more b/w than y.
This is ambigous because that is only correct when the effective values are
different. x=11 and y=12 with a granularity of 10 are resulting in the same
throttling.
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -5,7 +5,6 @@
>
> #include
> #include
> -
> #include
This white space is there on purpose to seperate linux and asm prefixed
includes. Stop making random white space changes.
> @@ -77,6 +76,8 @@ struct rftype {
> * @no_ctrl: Specifies max cache cbm or min mem b/w delay.
> * @min_cbm_bits:Minimum number of consecutive bits to be set
> * in a cache bit mask
> + * @info_files: resctrl info files for the resource
> + * @infofiles_len: Number of info files
This wants to be a seperate patch as it changes the way how the info files
are set up. The implementation for the MBA stuff is a follow up patch.
> * @max_delay: Max throttle delay
> * @delay_gran: Throttle delay granularity
> * @delay_linear:true if delay is in linear scale
> @@ -98,6 +99,8 @@ struct rdt_resource {
> int cbm_len;
> int min_cbm_bits;
> u32 no_ctrl;
> + struct rftype *info_files;
> + int infofiles_len;
> u32 max_delay;
> u32 delay_gran;
> u32 delay_linear;
> @@ -137,6 +140,9 @@ struct msr_param {
> int high;
> };
>
> +void rdt_get_cache_infofile(struct rdt_resource *r);
> +void rdt_get_mbe_infofile(struct rdt_resource *r);
> +
> extern struct mutex rdtgroup_mutex;
>
> extern struct rdt_resource rdt_resources_all[];
> diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
> index 6736e1d..bdfbd1d 100644
> --- a/arch/x86/kernel/cpu/intel_rdt.c
> +++ b/arch/x86/kernel/cpu/intel_rdt.c
> @@ -154,6 +154,7 @@ static void rdt_get_mem_config(struct rdt_resource *r)
> if (r->delay_linear)
> r->delay_gran = MAX_MBA_THRTL - r->max_delay;
>
> + rdt_get_mbe_infofile(r);
> r->capable = true;
> r->enabled = true;
> }
> @@ -168,6 +169,7 @@ static void rdt_get_cache_config(int idx, struct
> rdt_resource *r)
> r->num_closid = edx.split.cos_max + 1;
> r->cbm_len = eax.split.cbm_len + 1;
> r->no_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
> + rdt_get_cache_infofile(r);
> r->capable = true;
> r->enabled = true;
> }
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 53f1917..9d9b7f4 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -517,9 +517,41 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file
> *of,
>
> return 0;
> }
> +static int rdt_max_delay_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->max_delay);
> +
> + return 0;
> +}
> +
> +static int rdt_delay_gran_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + if (r->delay_linear)
> + seq_printf(seq, "%d\n", r->delay_gran);
> + else
> + seq_printf(seq, "power of 2\n");
> +
> + return 0;
> +}
> +
> +static int rdt_delay_linear_show(struct kernfs_open_file *of,
> + struct seq_file *seq, void *v)
> +{
> + struct rdt_resource *r = of->kn->parent->priv;
> +
> + seq_printf(seq, "%d\n", r->delay_linear);
> +
> + return 0;
> +}
>
> /* rdtgroup information files for one cache resource. */
> -static struct rftype res_info_files[] = {
> +static struct rftype res_cache_info_files[] = {
> {
> .name = "num_closids",
> .mode = 0444,
> @@ -540,11 +572,52 @@ static int rdt_min_cbm_bits_show(struct
> kernfs_open_file *of,
> },
> };
>
> +/* rdtgroup
