lkml.org 
[lkml]   [2015]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/7] x86/intel_rdt: Implement scheduling support for Intel RDT


On Sat, 2 May 2015, Peter Zijlstra wrote:

> On Fri, May 01, 2015 at 06:36:38PM -0700, Vikas Shivappa wrote:
>> Adds support for IA32_PQR_ASSOC MSR writes during task scheduling.
>>
>> The high 32 bits in the per processor MSR IA32_PQR_ASSOC represents the
>> CLOSid. During context switch kernel implements this by writing the
>> CLOSid of the cgroup to which the task belongs to the CPU's
>> IA32_PQR_ASSOC MSR.
>>
>> For Cache Allocation, this would let the task fill in the cache 'subset'
>> represented by the cgroup's Cache bit mask(CBM).
>>
>
> Are you guys for real? Have you even looked at the trainwreck this
> makes?
>
>> +static inline bool rdt_enabled(void)
>> +{
>> + return static_key_false(&rdt_enable_key);
>> +}
>
>> +/*
>> + * rdt_sched_in() - Writes the task's CLOSid to IA32_PQR_MSR
>> + * if the current Closid is different than the new one.
>> + */
>> +static inline void rdt_sched_in(struct task_struct *task)
>> +{
>> + struct intel_rdt *ir;
>> + unsigned int clos;
>> +
>> + if (!rdt_enabled())
>> + return;
>> +
>> + /*
>> + * This needs to be fixed
>> + * to cache the whole PQR instead of just CLOSid.
>> + * PQR has closid in high 32 bits and CQM-RMID in low 10 bits.
>> + * Should not write a 0 to the low 10 bits of PQR
>> + * and corrupt RMID.
>> + */
>> + clos = this_cpu_read(x86_cpu_clos);
>> +
>> + rcu_read_lock();
>> + ir = task_rdt(task);
>> + if (ir->clos == clos) {
>> + rcu_read_unlock();
>> + return;
>> + }
>> +
>> + wrmsr(MSR_IA32_PQR_ASSOC, 0, ir->clos);
>> + this_cpu_write(x86_cpu_clos, ir->clos);
>> + rcu_read_unlock();
>> +}
>
> You inject _ALL_ that into the scheduler hot path. Insane much?

At some point I had a #ifdef for the rdt_sched_in , will fix this.

>
>> +
>> +#else
>> +
>> +static inline void rdt_sched_in(struct task_struct *task) {}
>> +
>> #endif
>> #endif
>> diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
>> index 751bf4b..82ef4b3 100644
>> --- a/arch/x86/include/asm/switch_to.h
>> +++ b/arch/x86/include/asm/switch_to.h
>> @@ -8,6 +8,9 @@ struct tss_struct;
>> void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
>> struct tss_struct *tss);
>>
>> +#include <asm/intel_rdt.h>
>> +#define post_arch_switch(current) rdt_sched_in(current)
>> +
>> #ifdef CONFIG_X86_32
>>
>> #ifdef CONFIG_CC_STACKPROTECTOR
>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index f9123a8..cacb490 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -2241,6 +2241,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
>> prev_state = prev->state;
>> vtime_task_switch(prev);
>> finish_arch_switch(prev);
>> + post_arch_switch(current);
>> perf_event_task_sched_in(prev, current);
>> finish_lock_switch(rq, prev);
>> finish_arch_post_lock_switch();
>
> Not a word in the Changelog on this hook; that's double fail.

will add the changelog. we want the current task which no other existing hook
provides.

Thanks,
Vikas

>
> Please _THINK_ before writing code.
>


\
 
 \ /
  Last update: 2015-05-04 21:21    [W:0.100 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site