linux-kernel - Re: [PATCH v1 05/20] x86/resctrl: Create mba_sc configuration in the rdt

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: <10e2bb45-30a8-a7ce-7005-f12b594b991d@arm.com>
Date:   Tue, 31 Aug 2021 17:24:09 +0100
From:   James Morse <james.morse@....com>
To:     Jamie Iles <jamie@...iainc.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Babu Moger <Babu.Moger@....com>,
        shameerali.kolothum.thodi@...wei.com,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        lcherian@...vell.com, bobo.shaobowang@...wei.com
Subject: Re: [PATCH v1 05/20] x86/resctrl: Create mba_sc configuration in the
 rdt_domain

Hi Jamie,

On 11/08/2021 17:32, Jamie Iles wrote:
> On Thu, Jul 29, 2021 at 10:35:55PM +0000, James Morse wrote:
>> To support resctrl's MBA software controller, the architecture must provide
>> a second configuration array to hold the mbps_val from user-space.
>>
>> This complicates the interface between the architecture code.
>>
>> Make the filesystem parts of resctrl create an array for the mba_sc
>> values when the struct resctrl_schema is created. The software controller
>> can be changed to use this, allowing the architecture code to only
>> consider the values configured in hardware.

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index cf0db0b7a5d0..185f9bb992d1 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2030,6 +2030,60 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>>  			     struct rdtgroup *prgrp,
>>  			     struct kernfs_node **mon_data_kn);
>>  
>> +static int mba_sc_domain_allocate(struct rdt_resource *res,
>> +				  struct rdt_domain *d)
>> +{
>> +	u32 num_closid = closid_free_map_len;
>> +	int cpu = cpumask_any(&d->cpu_mask);
>> +	int i;
>> +
>> +	d->mba_sc = kcalloc_node(num_closid, sizeof(*d->mba_sc),
>> +				 GFP_KERNEL, cpu_to_node(cpu));
>> +	if (!d->mba_sc)
>> +		return -ENOMEM;
> 
> If a CPU was hotplugged before resctrl is mounted then isn't it possible 
> for this to already be allocated?  I might be misunderstanding the flows 
> here though...

Yeah, its tortuous. All this is behind an is_mba_sc(r), check, which can only return true
while the filesystem is mounted. cpus_read_lock() is what ensures the mount-time setup
doesn't race with the hotplug callbacks.


>> +	for (i = 0; i < num_closid; i++)
>> +		d->mba_sc[i].mbps_val = MBA_MAX_MBPS;
>> +
>> +	return 0;
>> +}
>> +
>> +static int mba_sc_allocate(struct rdt_resource *r)
>> +{
>> +	struct rdt_domain *d;
>> +	int ret;
>> +
>> +	lockdep_assert_cpus_held();
>> +
>> +	if (!is_mba_sc(r))
>> +		return 0;
>> +
>> +	list_for_each_entry(d, &r->domains, list) {
>> +		ret = mba_sc_domain_allocate(r, d);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}


>> @@ -3287,6 +3353,9 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d)
>>  		}
>>  	}
>>  
>> +	if (is_mba_sc(r))
>> +		mba_sc_domain_allocate(r, d);
> 
> This looks to be missing an error check.

Fixed, thanks.

James