Re: [PATCH 4/7] x86/intel_rdt: Implement scheduling support for Intel RDT

[Vikas Shivappa]

On Fri, 15 May 2015, Thomas Gleixner wrote:

> 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.

Ok , can fix both items in your comments. Reason I had it seperately is that the 
cache affects both cmt and cache allocation patches.

> Thanks,
>
> 	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/