public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Tero Kristo <tero.kristo@linux.intel.com>,
	 Hans de Goede <hdegoede@redhat.com>,
	platform-driver-x86@vger.kernel.org,
	 LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] Documentation: admin-guide: pm: Add efficiency vs. latency tradeoff to uncore documentation
Date: Tue, 27 Aug 2024 11:08:14 +0300 (EEST)	[thread overview]
Message-ID: <1b93f71a-8a36-f95e-86b9-2b8f330847ff@linux.intel.com> (raw)
In-Reply-To: <4d6adc49f295ad1dec26cd1a67ec3997686db4a9.camel@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 5591 bytes --]

On Mon, 26 Aug 2024, srinivas pandruvada wrote:

> On Fri, 2024-08-23 at 15:28 +0300, Ilpo Järvinen wrote:
> > On Wed, 21 Aug 2024, Tero Kristo wrote:
> > 
> > > Added documentation about the functionality of efficiency vs.
> > > latency tradeoff
> > > control in intel Xeon processors, and how this is configured via
> > > sysfs.
> > > 
> > > Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
> > > ---
> > >  .../pm/intel_uncore_frequency_scaling.rst     | 51
> > > +++++++++++++++++++
> > >  1 file changed, 51 insertions(+)
> > > 
> > > diff --git a/Documentation/admin-
> > > guide/pm/intel_uncore_frequency_scaling.rst b/Documentation/admin-
> > > guide/pm/intel_uncore_frequency_scaling.rst
> > > index 5ab3440e6cee..fb83aa2b744e 100644
> > > --- a/Documentation/admin-
> > > guide/pm/intel_uncore_frequency_scaling.rst
> > > +++ b/Documentation/admin-
> > > guide/pm/intel_uncore_frequency_scaling.rst
> > > @@ -113,3 +113,54 @@ to apply at each uncore* level.
> > >  
> > >  Support for "current_freq_khz" is available only at each fabric
> > > cluster
> > >  level (i.e., in uncore* directory).
> > > +
> > > +Efficiency vs. Latency Tradeoff
> > 
> > Does this section even cover the "tradeoff" part in its body? Why not
> > call 
> > it directly "Control" after ELC?
> > 
> > > +-------------------------------
> > > +
> > > +In the realm of high-performance computing, particularly with Xeon
> > > +processors, managing uncore frequency is an important aspect of
> > > system
> > > +optimization. Traditionally, the uncore frequency is ramped up
> > > rapidly
> > > +in high load scenarios. While this strategy achieves low latency,
> > > which
> > > +is crucial for time-sensitive computations, it does not
> > > necessarily yield
> > > +the best performance per watt, —a key metric for energy efficiency
> > > and
> > > +operational cost savings.
> > 
> > This entire paragraph feels more prose or history book than
> > documentation 
> > text. I'd suggest using something that goes more directly into the
> > point
> > about what ELC brings to the table (I suppose the goal is
> > "performance 
> > per watt" optimization, even that goal is only implied by the current
> > text, not explicitly stated as the goal here).
> > 
> 
> What about this?
> 
> Traditionally, the uncore frequency is ramped up to reach the maximum 
> possible level based on parameters like EPB (Energy perf Bias) and
> other system power management settings programmed by BIOS.  While this
> strategy achieves low latency for latency sensitive applications, it
> does not necessarily yield the best performance per watt. 

This again starts with a wrong foot. Don't use words like "traditionally",
"in the past", "historically", "is added", etc. that refer to past time
in documentation text at all. The premise with documentation for feature x 
is that the feature x exists. After these patches have been accepted, the 
reality is that ELC exists and time before does not matter so we don't 
encumber documentation text with that era that has become irrelevant.

You might occasionally have to mention what is not possible without ELC 
in case it's still possible to run stuff without ELC but don't put time 
references to it. However, it's not something you should start with in
the documentation text.

> The Efficiency Latency Control (ELC) feature is added to improve

"is added to improve" -> "improves"

> performance per watt. With this feature hardware power management
> algorithms optimize trade-off between latency and power consumption.
> But for some latency sensitive workloads further tuning can be done
> from OS to get desired performance.

I'd just start with this paragraph. It goes straight into the point and 
is good in that it tries to summarize what ELC tries to achieve.

> The hardware monitors the average CPU utilization across all cores

hardware or ELC-capable HW?

> in a power domain at regular intervals and decides a uncore frequency. 

This kind of feels something that belongs to the first paragraph if it's 
about ELC. (I'm left slightly unsure if ELC refers only to those controls 
mentioned below, or if it is the automatic uncore freq control plus the 
manual controls. I assume it's the latter because of "with this feature 
hardware power management algorithms optimize" sentence.)

> While this may result in the best performance per watt, workload may be
> expecting higher performance at the expense of power. Consider an
> application that intermittently wakes up to perform memory reads on an
> otherwise idle system. In such cases, if hardware lowers uncore
> frequency, then there may be delay in ramp up of frequency to meet
> target performance. 
>
> The ELC control defines some parameters which can be changed from OS.
> If the average CPU utilization is below a user defined threshold
> (elc_low_threshold_percent attribute below), the user defined uncore
> frequency floor frequency will be used (elc_floor_freq_khz attribute 
> below) instead of hardware calculated minimum. 
>
> Similarly in high load scenario where the CPU utilization goes above 
> the high threshold value (elc_high_threshold_percent attribute below) 
> instead of jumping to maximum uncore frequency, uncore frequency is 
> increased in 100MHz steps until the power limit is reached.
>
> Attributes for efficiency latency control: 
> .. 
> .. 

There were a few spaces at the end if lines, those should be removed.

-- 
 i.

  reply	other threads:[~2024-08-27  8:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-21 13:10 [PATCH 0/3] platform/x86: Add support for Intel uncore ELC feature Tero Kristo
2024-08-21 13:10 ` [PATCH 1/3] Documentation: admin-guide: pm: Add efficiency vs. latency tradeoff to uncore documentation Tero Kristo
2024-08-23 12:28   ` Ilpo Järvinen
2024-08-26 14:45     ` srinivas pandruvada
2024-08-27  8:08       ` Ilpo Järvinen [this message]
2024-08-27 11:30         ` srinivas pandruvada
2024-08-27 13:34           ` Ilpo Järvinen
2024-08-21 13:10 ` [PATCH 2/3] platform/x86/intel-uncore-freq: Add support for efficiency latency control Tero Kristo
2024-08-23 12:48   ` Ilpo Järvinen
2024-08-26 15:55     ` srinivas pandruvada
2024-08-21 13:10 ` [PATCH 3/3] platform/x86/intel-uncore-freq: Add efficiency latency control to sysfs interface Tero Kristo
2024-08-23 13:03   ` Ilpo Järvinen
2024-08-23 13:12     ` srinivas pandruvada
2024-08-23 13:29       ` Ilpo Järvinen
2024-08-23 14:43         ` srinivas pandruvada
2024-08-26  9:37           ` Ilpo Järvinen

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=1b93f71a-8a36-f95e-86b9-2b8f330847ff@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=tero.kristo@linux.intel.com \
    /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