linux-kernel - Re: [PATCH v9 24/26] x86/resctrl: Update assignments on event configuration changes

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: <50a7d4a1-f45a-427e-bfdb-980ac796238a@amd.com>
Date: Fri, 22 Nov 2024 14:12:11 -0600
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, babu.moger@....com,
 corbet@....net, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com
Cc: fenghua.yu@...el.com, x86@...nel.org, hpa@...or.com, thuth@...hat.com,
 paulmck@...nel.org, rostedt@...dmis.org, akpm@...ux-foundation.org,
 xiongwei.song@...driver.com, pawan.kumar.gupta@...ux.intel.com,
 daniel.sneddon@...ux.intel.com, perry.yuan@....com, sandipan.das@....com,
 kai.huang@...el.com, xiaoyao.li@...el.com, seanjc@...gle.com,
 jithu.joseph@...el.com, brijesh.singh@....com, xin3.li@...el.com,
 ebiggers@...gle.com, andrew.cooper3@...rix.com, mario.limonciello@....com,
 james.morse@....com, tan.shaopeng@...itsu.com, tony.luck@...el.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 peternewman@...gle.com, maciej.wieczor-retman@...el.com, eranian@...gle.com,
 jpoimboe@...nel.org, thomas.lendacky@....com
Subject: Re: [PATCH v9 24/26] x86/resctrl: Update assignments on event
 configuration changes

Hi Reinette,

On 11/21/2024 2:58 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 11/20/24 6:14 PM, Moger, Babu wrote:
>> On 11/18/2024 1:43 PM, Reinette Chatre wrote:
>>> On 10/29/24 4:21 PM, Babu Moger wrote:
> 
>>>> +static void resctrl_arch_update_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
>>>> +                     enum resctrl_event_id evtid, u32 rmid,
>>>> +                     u32 closid, u32 cntr_id, u32 val)
>>>> +{
>>>> +    union l3_qos_abmc_cfg abmc_cfg = { 0 };
>>>> +
>>>> +    abmc_cfg.split.cfg_en = 1;
>>>> +    abmc_cfg.split.cntr_en = 1;
>>>> +    abmc_cfg.split.cntr_id = cntr_id;
>>>> +    abmc_cfg.split.bw_src = rmid;
>>>> +    abmc_cfg.split.bw_type = val;
>>>> +
>>>> +    wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, abmc_cfg.full);
>>>
>>> Is it needed to create an almost duplicate function? What if instead
>>> only resctrl_arch_config_cntr() exists and it uses parameter to decide
>>> whether to call resctrl_abmc_config_one_amd() directly or via
>>> smp_call_function_any()? I think that should help to make clear how
>>> the code flows.
>>> Also note that this is an almost identical arch callback with no
>>> error return. I expect that building on existing resctrl_arch_config_cntr()
>>> will make things easier to understand.
>>
>> It can be done. But it takes another parameter to the function.
>> It has 7 parameters already. This will be 8th.
>> Will change it if that is ok.
> 
> Please correct me if I am wrong but I am not familiar with a restriction on number
> of parameters. It seems unnecessary to me to create two almost duplicate 7 parameter
> functions to avoid one 8 parameter function.

I dont see any hard requirement. Will add one parameter for smp call or 
direct call.
> 
>>> Since MBM_EVENT_ARRAY_INDEX is a macro it can be called closer to where it is used,
>>> within  rdtgroup_find_grp_by_cntr_id_index(), which prompts a reconsider of that function name.
>>
>>
>> How about ?
>>
>> static struct rdtgroup *rdtgroup_find_grp_by_cntr_id_event(int cntr_id, enum resctrl_event_id evtid)
> 
> ... or for something shorter just get_rdtgroup_from_cntr_event(), but no hard requirement.
> 
Sure
Thanks
-- 
- Babu Moger