lkml.org 
[lkml]   [2017]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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 = &res_mbe_info_files[0];
>
> See other mail.
>

Will fix the MBE to MBA and the pointer init.

Thanks,
Vikas

> Thanks,
>
> tglx
>

\
 
 \ /
  Last update: 2017-03-11 00:26    [W:0.051 / U:16.544 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site