by Shivappa Vikas

[permalink] [raw]

Subject: Re: [PATCH 3/7] x86/intel_rdt: Support cache bit mask for Intel CAT

On Thu, 9 Apr 2015, Marcelo Tosatti wrote:

> On Thu, Mar 12, 2015 at 04:16:03PM -0700, Vikas Shivappa wrote:
>> Add support for cache bit mask manipulation. The change adds a file to
>> the RDT cgroup which represents the CBM(cache bit mask) for the cgroup.
>>
>> The RDT cgroup follows cgroup hierarchy ,mkdir and adding tasks to the
>> cgroup never fails. When a child cgroup is created it inherits the
>> CLOSid and the CBM from its parent. When a user changes the default
>> CBM for a cgroup, a new CLOSid may be allocated if the CBM was not
>> used before. If the new CBM is the one that is already used, the
>> count for that CLOSid<->CBM is incremented. The changing of 'cbm'
>> may fail with -ENOSPC once the kernel runs out of maximum CLOSids it
>> can support.
>> User can create as many cgroups as he wants but having different CBMs
>> at the same time is restricted by the maximum number of CLOSids
>> (multiple cgroups can have the same CBM).
>> Kernel maintains a CLOSid<->cbm mapping which keeps count
>> of cgroups using a CLOSid.
>>
>> The tasks in the CAT cgroup would get to fill the LLC cache represented
>> by the cgroup's 'cbm' file.
>>
>> Reuse of CLOSids for cgroups with same bitmask also has following
>> advantages:
>> - This helps to use the scant CLOSids optimally.
>> - This also implies that during context switch, write to PQR-MSR is done
>> only when a task with a different bitmask is scheduled in.
>>
>> Signed-off-by: Vikas Shivappa <[email protected]>
>> ---
>> arch/x86/include/asm/intel_rdt.h | 3 +
>> arch/x86/kernel/cpu/intel_rdt.c | 205 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 208 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
>> index 87af1a5..0ed28d9 100644
>> --- a/arch/x86/include/asm/intel_rdt.h
>> +++ b/arch/x86/include/asm/intel_rdt.h
>> @@ -4,6 +4,9 @@
>> #ifdef CONFIG_CGROUP_RDT
>>
>> #include <linux/cgroup.h>
>> +#define MAX_CBM_LENGTH 32
>> +#define IA32_L3_CBM_BASE 0xc90
>> +#define CBM_FROM_INDEX(x) (IA32_L3_CBM_BASE + x)
>>
>> struct rdt_subsys_info {
>> /* Clos Bitmap to keep track of available CLOSids.*/
>> diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
>> index 3726f41..495497a 100644
>> --- a/arch/x86/kernel/cpu/intel_rdt.c
>> +++ b/arch/x86/kernel/cpu/intel_rdt.c
>> @@ -33,6 +33,9 @@ static struct rdt_subsys_info rdtss_info;
>> static DEFINE_MUTEX(rdt_group_mutex);
>> struct intel_rdt rdt_root_group;
>>
>> +#define rdt_for_each_child(pos_css, parent_ir) \
>> + css_for_each_child((pos_css), &(parent_ir)->css)
>> +
>> static inline bool cat_supported(struct cpuinfo_x86 *c)
>> {
>> if (cpu_has(c, X86_FEATURE_CAT_L3))
>> @@ -83,6 +86,31 @@ static int __init rdt_late_init(void)
>> late_initcall(rdt_late_init);
>>
>> /*
>> + * Allocates a new closid from unused closids.
>> + * Called with the rdt_group_mutex held.
>> + */
>> +
>> +static int rdt_alloc_closid(struct intel_rdt *ir)
>> +{
>> + unsigned int id;
>> + unsigned int maxid;
>> +
>> + lockdep_assert_held(&rdt_group_mutex);
>> +
>> + maxid = boot_cpu_data.x86_cat_closs;
>> + id = find_next_zero_bit(rdtss_info.closmap, maxid, 0);
>> + if (id == maxid)
>> + return -ENOSPC;
>> +
>> + set_bit(id, rdtss_info.closmap);
>> + WARN_ON(ccmap[id].cgrp_count);
>> + ccmap[id].cgrp_count++;
>> + ir->clos = id;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> * Called with the rdt_group_mutex held.
>> */
>> static int rdt_free_closid(struct intel_rdt *ir)
>> @@ -133,8 +161,185 @@ static void rdt_css_free(struct cgroup_subsys_state *css)
>> mutex_unlock(&rdt_group_mutex);
>> }
>>
>> +/*
>> + * Tests if atleast two contiguous bits are set.
>> + */
>> +
>> +static inline bool cbm_is_contiguous(unsigned long var)
>> +{
>> + unsigned long first_bit, zero_bit;
>> + unsigned long maxcbm = MAX_CBM_LENGTH;
>> +
>> + if (bitmap_weight(&var, maxcbm) < 2)
>> + return false;
>> +
>> + first_bit = find_next_bit(&var, maxcbm, 0);
>> + zero_bit = find_next_zero_bit(&var, maxcbm, first_bit);
>> +
>> + if (find_next_bit(&var, maxcbm, zero_bit) < maxcbm)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static int cat_cbm_read(struct seq_file *m, void *v)
>> +{
>> + struct intel_rdt *ir = css_rdt(seq_css(m));
>> +
>> + seq_printf(m, "%08lx\n", ccmap[ir->clos].cbm);
>> + return 0;
>> +}
>> +
>> +static int validate_cbm(struct intel_rdt *ir, unsigned long cbmvalue)
>> +{
>> + struct intel_rdt *par, *c;
>> + struct cgroup_subsys_state *css;
>> + unsigned long *cbm_tmp;
>> +
>> + if (!cbm_is_contiguous(cbmvalue)) {
>> + pr_info("cbm should have >= 2 bits and be contiguous\n");
>> + return -EINVAL;
>> + }
>> +
>> + par = parent_rdt(ir);
>> + cbm_tmp = &ccmap[par->clos].cbm;
>> + if (!bitmap_subset(&cbmvalue, cbm_tmp, MAX_CBM_LENGTH))
>> + return -EINVAL;
>
> Can you have different errors for the different cases?

Could use -EPER

>
>> + rcu_read_lock();
>> + rdt_for_each_child(css, ir) {
>> + c = css_rdt(css);
>> + cbm_tmp = &ccmap[c->clos].cbm;
>> + if (!bitmap_subset(cbm_tmp, &cbmvalue, MAX_CBM_LENGTH)) {
>> + pr_info("Children's mask not a subset\n");
>> + rcu_read_unlock();
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + rcu_read_unlock();
>> + return 0;
>> +}
>> +
>> +static bool cbm_search(unsigned long cbm, int *closid)
>> +{
>> + int maxid = boot_cpu_data.x86_cat_closs;
>> + unsigned int i;
>> +
>> + for (i = 0; i < maxid; i++)
>> + if (bitmap_equal(&cbm, &ccmap[i].cbm, MAX_CBM_LENGTH)) {
>> + *closid = i;
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static void cbmmap_dump(void)
>> +{
>> + int i;
>> +
>> + pr_debug("CBMMAP\n");
>> + for (i = 0; i < boot_cpu_data.x86_cat_closs; i++)
>> + pr_debug("cbm: 0x%x,cgrp_count: %u\n",
>> + (unsigned int)ccmap[i].cbm, ccmap[i].cgrp_count);
>> +}
>> +
>> +static void cpu_cbm_update(void *info)
>> +{
>> + unsigned int closid = *((unsigned int *)info);
>> +
>> + wrmsrl(CBM_FROM_INDEX(closid), ccmap[closid].cbm);
>> +}
>> +
>> +static inline void cbm_update(unsigned int closid)
>> +{
>> + int pkg_id = -1;
>> + int cpu;
>> +
>> + for_each_online_cpu(cpu) {
>> + if (pkg_id == topology_physical_package_id(cpu))
>> + continue;
>> + smp_call_function_single(cpu, cpu_cbm_update, &closid, 1);
>> + pkg_id = topology_physical_package_id(cpu);
>> +
>> +
>
> Can use smp_call_function_many, once, more efficient.
>
> Can this race with CPU hotplug? BTW, on CPU hotplug, where are
> the IA32_L3_MASK_n initialized for the new CPU ?
>

Thanks for pointing out , Will fix this . Think i was terrible when i changed
the design to not use
the cpuset did not change the hot cpu update , I remembered an other similar
update needed.The s3 resume needs a fix to the software cache as we used the msr before.

Thanks,
Vikas

>