Messages in this thread | | | Date | Mon, 4 May 2015 11:39:21 -0700 (PDT) | From | Vikas Shivappa <> | Subject | Re: [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. >
| |