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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1701231121460.3301@vshiva-Udesk>
Date:   Mon, 23 Jan 2017 11:45:41 -0800 (PST)
From:   Shivappa Vikas <vikas.shivappa@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
cc:     Shivappa Vikas <vikas.shivappa@...el.com>,
        Vikas Shivappa <vikas.shivappa@...ux.intel.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org, hpa@...or.com,
        mingo@...nel.org, peterz@...radead.org, ravi.v.shankar@...el.com,
        tony.luck@...el.com, fenghua.yu@...el.com, h.peter.anvin@...el.com
Subject: 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		= &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
>
>