Re: [PATCH 6/8] x86/intel_rdt/mba: Add info directory files for MBA

[Shivappa Vikas <vikas.shivappa@intel.com>]

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		= &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.
>>
>> 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
>
>