From: Tim Chen <tim.c.chen@linux.intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
rjw@rjwysocki.net, mingo@redhat.com, bp@suse.de, x86@kernel.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, peterz@infradead.org,
jolsa@redhat.com
Subject: Re: [PATCH v5 5/9] x86/sysctl: Add sysctl for ITMT scheduling feature
Date: Thu, 06 Oct 2016 10:37:46 -0700 [thread overview]
Message-ID: <1475775466.3916.319.camel@linux.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1610061307030.22338@nanos>
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
next prev parent reply other threads:[~2016-10-06 17:37 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-01 11:45 [PATCH v5 0/9] Support Intel® Turbo Boost Max Technology 3.0 Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 1/9] sched: Extend scheduler's asym packing Srinivas Pandruvada
2016-10-01 16:38 ` Nilay Vaish
2016-10-03 16:49 ` Tim Chen
2016-10-01 11:45 ` [PATCH v5 2/9] x86/topology: Provide topology_num_packages() Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 3/9] x86/topology: Define x86's arch_update_cpu_topology Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 4/9] x86: Enable Intel Turbo Boost Max Technology 3.0 Srinivas Pandruvada
2016-10-05 14:23 ` Thomas Gleixner
2016-10-05 16:05 ` Tim Chen
2016-10-01 11:45 ` [PATCH v5 5/9] x86/sysctl: Add sysctl for ITMT scheduling feature Srinivas Pandruvada
2016-10-05 14:35 ` Thomas Gleixner
2016-10-05 16:24 ` Tim Chen
2016-10-06 11:13 ` Thomas Gleixner
2016-10-06 17:37 ` Tim Chen [this message]
2016-10-12 16:50 ` Tim Chen
2016-10-01 11:45 ` [PATCH v5 6/9] x86/sched: Add SD_ASYM_PACKING flags to x86 ITMT CPU Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 7/9] acpi: bus: Enable HWP CPPC objects Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 8/9] acpi: bus: Set _OSC for diverse core support Srinivas Pandruvada
2016-10-01 11:45 ` [PATCH v5 9/9] cpufreq: intel_pstate: Use CPPC to get max performance Srinivas Pandruvada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1475775466.3916.319.camel@linux.intel.com \
--to=tim.c.chen@linux.intel.com \
--cc=bp@suse.de \
--cc=jolsa@redhat.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).