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

[Shivappa Vikas]

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