linux-kernel - Re: [PATCH V4 1/4] x86/resctrl: Enable user to view and select thread throttling mode

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] [day] [month] [year] [list]

Date:   Mon, 18 May 2020 15:46:02 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Babu Moger <babu.moger@....com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "fenghua.yu@...el.com" <fenghua.yu@...el.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "tony.luck@...el.com" <tony.luck@...el.com>
Cc:     "kuo-lang.tseng@...el.com" <kuo-lang.tseng@...el.com>,
        "ravi.v.shankar@...el.com" <ravi.v.shankar@...el.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4 1/4] x86/resctrl: Enable user to view and select thread
 throttling mode

Hi Babu,

On 5/18/2020 10:28 AM, Babu Moger wrote:
> Hi Reinette,
>  Looks good most part. Few minor comments.

Thank you very much for taking a look.

> 
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@...el.com>
>> Sent: Saturday, May 16, 2020 1:29 PM
>> To: tglx@...utronix.de; fenghua.yu@...el.com; bp@...en8.de;
>> tony.luck@...el.com
>> Cc: kuo-lang.tseng@...el.com; ravi.v.shankar@...el.com; mingo@...hat.com;
>> Moger, Babu <Babu.Moger@....com>; hpa@...or.com; x86@...nel.org;
>> linux-kernel@...r.kernel.org; Reinette Chatre <reinette.chatre@...el.com>
>> Subject: [PATCH V4 1/4] x86/resctrl: Enable user to view and select thread
>> throttling mode
>>

...

>> +static void mba_cfg_reconfigure_throttle_mode(struct rdt_resource *r)
>> +{
>> +	if (!r->alloc_capable)
>> +		return;
>> +
>> +	if (r == &rdt_resources_all[RDT_RESOURCE_MBA] &&
>> +	    r->membw.arch_throttle_mode == THREAD_THROTTLE_MIN_MAX)
>> +		wrmsrl(MSR_MBA_CFG, mba_cfg_msr);
>> +}
> 
> How about this? It is kind of consistent with other checks that are done.
> 
> If (r->alloc_capable && (r == &rdt_resources_all[RDT_RESOURCE_MBA]) &&
>     (r->membw.arch_throttle_mode == THREAD_THROTTLE_MIN_MAX))
>              wrmsrl(MSR_MBA_CFG, mba_cfg_msr);
> 

Sure. Will do (with fewer parentheses).

>> +
>> +/*
>> + * Model-specific test to determine if platform where memory bandwidth
>> + * control is applied to a core can be configured to apply either the
>> + * maximum or minimum of the per-thread delay values.
>> + * By default, platforms where memory bandwidth control is applied to a
>> + * core will select the maximum delay value of the per-thread CLOS.
>> + *
>> + * NOTE: delay value programmed to hardware is inverse of bandwidth
>> + * percentage configured via user interface.
>> + */
>> +static bool mba_cfg_supports_min_max_intel(void)
>> +{
>> +	switch (boot_cpu_data.x86_model) {
>> +	case INTEL_FAM6_ATOM_TREMONT_D:
>> +	case INTEL_FAM6_ICELAKE_X:
>> +	case INTEL_FAM6_ICELAKE_D:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +
>> +	return false;
> 
> Is this last return required?  I don't think so.  We will never go here.
> 

Indeed. Thank you for catching this.

Reinette