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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html