Re: [PATCH v5 5/9] x86/sysctl: Add sysctl for ITMT scheduling feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2016-10-06 at 13:13 +0200, Thomas Gleixner wrote:
> On Wed, 5 Oct 2016, Tim Chen wrote:
> > 
> > On Wed, 2016-10-05 at 16:35 +0200, Thomas Gleixner wrote:
> > > 
> > > > 
> > > > +	if (itmt_supported) {
> > > > +		itmt_sysctl_header =
> > > > +			register_sysctl_table(itmt_root_table);
> > > > +		if (!itmt_sysctl_header) {
> > > > +			mutex_unlock(&itmt_update_mutex);
> > > > +			return;
> > > So you now have a state of capable which cannot be enabled. Whats the
> > > point?
> > For multi-socket system where ITMT is not enabled by default, the operator
> > can still decide to enable it via sysctl.
> With a sysctl which failed to be installed. Good luck with that.

I misunderstood your earlier comment.
You are talking about the case where we fail to register the sysctl?

In this case, the system is in a state that indicates it is 
ITMT capable but cannot be enabled.  So we return and do not turn on ITMT
scheduling.  The system operator should always have the capability
to enable/disable ITMT via sysctl.  So we do not turn on ITMT if operator has
no control over it, even if the system is capable of ITMT.


>  
> > 
> > > 
> > > > 
> > > > +		}
> > > > +		/*
> > > > +		 * ITMT capability automatically enables ITMT
> > > > +		 * scheduling for small systems (single node).
> > > > +		 */
> > > > +		if (topology_num_packages() == 1)
> > > > +			sysctl_sched_itmt_enabled = 1;
> > > > +	} else {
> > > > +		if (itmt_sysctl_header)
> > > > +			unregister_sysctl_table(itmt_sysctl_header);
> > > > +	}
> > > > +
> > > > +	if (sysctl_sched_itmt_enabled) {
> > > > +		/* disable sched_itmt if we are no longer ITMT capable */
> > > > +		if (!itmt_supported)
> > > How do you get here if itmt is not supported? 
> > If the OS decides to turn off ITMT for any reason, (i.e. invoke 
> > sched_set_itmt_support(false) after it has turned on itmt_support
> > before), this is the logic to do it.  We don't turn off ITMT support
> > after it has been turned on today, in the future the OS may.
> Then please make this two functions (set/clear) so one can actually follow
> the logic. The above is just too convoluted.

Sure, I will add a clear function and move the clearing logic there.

Thanks.

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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux