linux-kernel - Re: [PATCH v2 05/23] x86/resctrl: Add domain online callback for resctrl work

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: <73573b55-f852-f9f4-ead8-a0dc498e9f40@arm.com>
Date:   Fri, 22 Oct 2021 19:30:12 +0100
From:   James Morse <james.morse@....com>
To:     Babu Moger <babu.moger@....com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Cc:     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>,
        shameerali.kolothum.thodi@...wei.com,
        Jamie Iles <jamie@...iainc.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        lcherian@...vell.com, bobo.shaobowang@...wei.com,
        tan.shaopeng@...itsu.com
Subject: Re: [PATCH v2 05/23] x86/resctrl: Add domain online callback for
 resctrl work

Hi Babu,

On 20/10/2021 00:19, Babu Moger wrote:
> On 10/1/21 11:02 AM, James Morse wrote:
>> Because domains are exposed to user-space via resctrl, the filesystem
>> must update its state when CPU hotplug callbacks are triggered.
>>
>> Some of this work is common to any architecture that would support
>> resctrl, but the work is tied up with the architecture code to
>> allocate the memory.
>>
>> Move domain_setup_mon_state(), the monitor subdir creation call and the
>> mbm/limbo workers into a new resctrl_online_domain() call. These bits
>> are not specific to the architecture. Grouping them in one function
>> allows that code to be moved to /fs/ and re-used by another architecture.

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index e243c7d15b81..19691f9ab061 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c

>> @@ -3236,6 +3233,64 @@ static int __init rdtgroup_setup_root(void)
>>  	return ret;
>>  }
>>  
>> +static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d)
>> +{
>> +	size_t tsize;
>> +
>> +	if (is_llc_occupancy_enabled()) {
>> +		d->rmid_busy_llc = bitmap_zalloc(r->num_rmid, GFP_KERNEL);
>> +		if (!d->rmid_busy_llc)
>> +			return -ENOMEM;
>> +	}
>> +	if (is_mbm_total_enabled()) {
>> +		tsize = sizeof(*d->mbm_total);
>> +		d->mbm_total = kcalloc(r->num_rmid, tsize, GFP_KERNEL);
>> +		if (!d->mbm_total) {
>> +			bitmap_free(d->rmid_busy_llc);
>> +			return -ENOMEM;
>> +		}
>> +	}
>> +	if (is_mbm_local_enabled()) {
>> +		tsize = sizeof(*d->mbm_local);
>> +		d->mbm_local = kcalloc(r->num_rmid, tsize, GFP_KERNEL);
>> +		if (!d->mbm_local) {
>> +			bitmap_free(d->rmid_busy_llc);
>> +			kfree(d->mbm_total);
>> +			return -ENOMEM;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d)
>> +{
>> +	int err;
>> +
>> +	lockdep_assert_held(&rdtgroup_mutex);

> Looks like lockdep_assert_held was not there in this sequence.
> Are you concerned about this lock not being held?

Its partly paranoia, partly documentation.

This is to document that the caller has to take this mutex, it protects the domain
pointers that are written in domain_setup_mon_state(), and read by __mon_event_count() and
the like.

I have a patch later in the tree that splits the locking done by the arch-code from the
locking done by the filesystem. Today those are both rdtgroup_mutex.



Thanks,

James