Messages in this thread | | | Date | Fri, 15 May 2015 21:39:05 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 4/7] x86/intel_rdt: Implement scheduling support for Intel RDT |
| |
On Mon, 11 May 2015, Vikas Shivappa wrote: > struct rdt_subsys_info { > /* Clos Bitmap to keep track of available CLOSids.*/ > @@ -24,6 +30,11 @@ struct clos_cbm_map { > unsigned int clos_refcnt; > }; > > +static inline bool rdt_enabled(void) > +{ > + return static_key_false(&rdt_enable_key);
So again, why this useless helper function for a single call site?
> +static inline void intel_rdt_sched_in(void) > +{ > + if (rdt_enabled()) > + __rdt_sched_in(); > +}
> +void __rdt_sched_in(void) > +{ > + struct task_struct *task = current; > + struct intel_rdt *ir; > + unsigned int clos; > + > + /* > + * 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.
And why is this not fixed __BEFORE__ this patch? You can do the changes to struct intel_cqm_state in a seperate patch and then do the proper implementation from the beginning instead of providing a half broken variant which gets replaced in the next patch.
Thanks,
tglx
| |