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 <linux/kernfs.h>
> #include <linux/jump_label.h>
> -
> #include <asm/intel_rdt_common.h>
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 information files for MBE. */
> +static struct rftype res_mbe_info_files[] = {
> + {
> + .name = "num_closids",
> + .mode = 0444,
> + .kf_ops = &rdtgroup_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.
Thanks,
tglx
