On Wed, 2016-10-26 at 12:49 +0200, Thomas Gleixner wrote:
> On Thu, 20 Oct 2016, Tim Chen wrote:
> >
> > +static int sched_itmt_update_handler(struct ctl_table *table, int write,
> > + void __user *buffer, size_t *lenp, loff_t *ppos)
> Please align the arguments proper
>
> static int
> sched_itmt_update_handler(struct ctl_table *table, int write,
> void __user *buffer, size_t *lenp, loff_t *ppos)
>
Okay.
> >
> > +{
> > + int ret;
> > + unsigned int old_sysctl;
> unsigned int old_sysctl;
> int ret;
>
> Please. It's way simpler to read.
Sure.
>
> >
> > -void sched_set_itmt_support(void)
> > +int sched_set_itmt_support(void)
> > {
> > mutex_lock(&itmt_update_mutex);
> >
> > + if (sched_itmt_capable) {
> > + mutex_unlock(&itmt_update_mutex);
> > + return 0;
> > + }
> > +
> > + itmt_sysctl_header = register_sysctl_table(itmt_root_table);
> > + if (!itmt_sysctl_header) {
> > + mutex_unlock(&itmt_update_mutex);
> > + return -ENOMEM;
> > + }
> > +
> > sched_itmt_capable = true;
> >
> > + /*
> > + * ITMT capability automatically enables ITMT
> > + * scheduling for small systems (single node).
> > + */
> > + if (topology_num_packages() == 1)
> > + sysctl_sched_itmt_enabled = 1;
> I really hate this. This is policy and the kernel should not impose
> policy. Why would I like to have this enforced on my single socket XEON
> server?
>
> >
> > + if (sysctl_sched_itmt_enabled) {
> Why would sysctl_sched_itmt_enabled be true at this point, aside of the
> above policy imposement?
That's true, it will only be enabled for the above case. I can merge
it into the if check above.
Tim
