Message-ID: <1b93f71a-8a36-f95e-86b9-2b8f330847ff@linux.intel.com>
Date: Tue, 27 Aug 2024 11:08:14 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
cc: Tero Kristo <tero.kristo@...ux.intel.com>, 
    Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] Documentation: admin-guide: pm: Add efficiency vs.
 latency tradeoff to uncore documentation

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@...ux.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.

Powered by blists - more mailing lists