[PATCH v3] lparstat: Fix Idle and busy PURR/SPURR

[Saket Kumar Bhaskar]

lparstat -E gives %busy and %idle for actual(PURR based) and normalized
(SPURR based).Idle and busy PURR/SPURR values are not adding upto 100%
in case of dedicated-donate and shared partitions, with the present
formula. Because of this, users might get a false impression of resource
utilisation. This is expected because a core can be in either
idle or busy state out of total of 100(core's shared resource can
either be consumed or be left idle). When lpar is in dedicated-donate
or shared,the purr values are not being counted when the CPU is ceded.
The idle_purr is calculated by taking snapshots of purr values at
every idle entry and idle exit. So, when a CPU is ceded, the calculation
for idle_purr will be wrong as purr is not being counted.

Before Change:
|-----------------------------------------------------------------|
| Dedicated-donate (8 cores) : |
|----------------------|---------------------|--------------------|
| | Actual | Normalized |
| Stress-ng threads |---------------------|--------------------|
| | %busy | %idle | %busy | %idle |
|----------------------|----------|----------|---------|----------|
| 0 threads | 0.02 | 0.05 | 0.02 | 0.05 |
|----------------------|----------|----------|---------|----------|
| 8 threads | 32.64 | 17.37 | 35.25 | 18.77 |
|----------------------|----------|----------|---------|----------|
| 16 threads | 58.61 | 16.42 | 63.29 | 17.74 |
|----------------------|----------|----------|---------|----------|
| 24 threads | 78.14 | 21.86 | 84.39 | 23.61 |
|----------------------|----------|----------|---------|----------|
| 32 threads | 83.60 | 16.40 | 90.30 | 17.71 |
|----------------------|----------|----------|---------|----------|
| 40 threads | 91.90 | 6.94 | 98.31 | 7.46 |
|----------------------|----------|----------|---------|----------|
| 48 threads | 96.08 | 3.92 | 102.79 | 4.21 |
|----------------------|----------|----------|---------|----------|
| 56 threads | 98.42 | 1.57 | 105.31 | 1.69 |
|----------------------|----------|----------|---------|----------|
| 64 threads | 100.00 | 0.00 | 106.00 | 0.00 |
|----------------------|----------|----------|---------|----------|

|-----------------------------------------------------------------|
| Shared Capped (8 VP / 4 EC) : |
|----------------------|---------------------|--------------------|
| | Actual | Normalized |
| Stress-ng threads |---------------------|--------------------|
| | %busy | %idle | %busy | %idle |
|----------------------|----------|----------|---------|----------|
| 0 threads | 0.04 | 0.18 | 0.03 | 0.19 |
|----------------------|----------|----------|---------|----------|
| 8 threads | 35.90 | 14.09 | 38.77 | 15.21 |
|----------------------|----------|----------|---------|----------|
| 16 threads | 35.25 | 14.84 | 38.08 | 16.02 |
|----------------------|----------|----------|---------|----------|
| 24 threads | 40.13 | 9.73 | 42.93 | 10.43 |
|----------------------|----------|----------|---------|----------|
| 32 threads | 44.13 | 5.73 | 47.22 | 6.14 |
|----------------------|----------|----------|---------|----------|
| 40 threads | 46.47 | 3.42 | 50.18 | 3.69 |
|----------------------|----------|----------|---------|----------|
| 48 threads | 48.03 | 1.83 | 51.39 | 1.96 |
|----------------------|----------|----------|---------|----------|
| 56 threads | 49.04 | 0.86 | 52.47 | 0.93 |
|----------------------|----------|----------|---------|----------|
| 64 threads | 49.87 | 0.00 | 53.36 | 0.00 |
|----------------------|----------|----------|---------|----------|

This commit, rather than considering delta_idle_purr for calculation of
idle ticks, takes (delta_tb - delta_purr + delta_idle_purr) as total
ticks for which the CPUs were idle. Here, since delta_idle_purr will
also contain some idle ticks, thats why it is added to the formula.
Since, the output was correct for dedicated capped mode, changes has
been made only for shared and dedicated-donate mode.
Further, no changes has been made for calculation of %busy.

Similar changes has been done for SPURR.

After Change:

|-----------------------------------------------------------------|
| Dedicated-donate (8 cores) : |
|----------------------|---------------------|--------------------|
| | Actual | Normalized |
| Stress-ng threads |---------------------|--------------------|
| | %busy | %idle | %busy | %idle |
|----------------------|----------|----------|---------|----------|
| 0 threads | 0.02 | 99.98 | 0.02 | 100.04 |
|----------------------|----------|----------|---------|----------|
| 8 threads | 35.97 | 64.03 | 38.84 | 61.51 |
|----------------------|----------|----------|---------|----------|
| 16 threads | 58.60 | 41.40 | 63.28 | 37.08 |
|----------------------|----------|----------|---------|----------|
| 24 threads | 78.14 | 21.86 | 84.39 | 23.61 |
|----------------------|----------|----------|---------|----------|
| 32 threads | 83.60 | 16.41 | 90.29 | 17.71 |
|----------------------|----------|----------|---------|----------|
| 40 threads | 92.96 | 7.04 | 100.39 | 7.61 |
|----------------------|----------|----------|---------|----------|
| 48 threads | 96.08 | 3.92 | 103.77 | 4.24 |
|----------------------|----------|----------|---------|----------|
| 56 threads | 98.42 | 1.58 | 105.31 | 1.68 |
|----------------------|----------|----------|---------|----------|
| 64 threads | 100.00 | 0.00 | 107.00 | 0.00 |
|----------------------|----------|----------|---------|----------|

|-----------------------------------------------------------------|
| Shared Capped (8 VP / 4 EC) : |
|----------------------|---------------------|--------------------|
| | Actual | Normalized |
| Stress-ng threads |---------------------|--------------------|
| | %busy | %idle | %busy | %idle |
|----------------------|----------|----------|---------|----------|
| 0 threads | 0.03 | 99.97 | 0.19 | 99.44 |
|----------------------|----------|----------|---------|----------|
| 8 threads | 35.91 | 64.09 | 38.78 | 61.58 |
|----------------------|----------|----------|---------|----------|
| 16 threads | 36.83 | 63.17 | 39.78 | 60.55 |
|----------------------|----------|----------|---------|----------|
| 24 threads | 40.16 | 59.84 | 43.37 | 56.95 |
|----------------------|----------|----------|---------|----------|
| 32 threads | 44.47 | 55.53 | 48.02 | 52.38 |
|----------------------|----------|----------|---------|----------|
| 40 threads | 46.55 | 53.45 | 50.27 | 50.04 |
|----------------------|----------|----------|---------|----------|
| 48 threads | 48.13 | 51.87 | 52.48 | 47.82 |
|----------------------|----------|----------|---------|----------|
| 56 threads | 49.01 | 50.99 | 52.93 | 47.41 |
|----------------------|----------|----------|---------|----------|
| 64 threads | 49.90 | 50.10 | 53.40 | 46.19 |
|----------------------|----------|----------|---------|----------|

Before Change:
%idle = delta_idle_purr / delta_tb * 100

After Change:
%idle = (delta_tb - delta_purr + delta_idle_purr) / delta_tb * 100

Signed-off-by: Saket Kumar Bhaskar <sk...@linux.ibm.com>
---

v2 -> v3:
- Added condition to contain error in delta_tb - delta_purr between
0% and -1%.
- Accomodated other comments from previous version.

v1: https://groups.google.com/g/powerpc-utils-devel/c/IZs0Norg8nc

v2: https://groups.google.com/g/powerpc-utils-devel/c/IZs0Norg8nc/m/UfDTCr8jBgAJ

src/lparstat.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/lparstat.c b/src/lparstat.c
index d2fdb3f..5bd2483 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -514,24 +514,43 @@ void get_cpu_util_purr(struct sysentry *unused_se, char *buf)
void get_cpu_idle_purr(struct sysentry *unused_se, char *buf)
{
double delta_tb, delta_purr, delta_idle_purr;
double physc, idle;
+ char *descr;
+ char mode[32];

delta_tb = get_scaled_tb();
delta_purr = get_delta_value("purr");
delta_idle_purr = get_delta_value("idle_purr");

+ get_sysdata("shared_processor_mode", &descr, mode);
+ if (!strcmp(mode, "Dedicated"))
+ get_sysdata("DedDonMode", &descr, mode);
+
/*
* Given that these values are read from different
* sources (purr from lparcfg and idle_purr from sysfs),
* a small variation in the values is possible.
* In such cases, round down delta_idle_purr to delta_purr.
*/
if (delta_idle_purr > delta_purr)
delta_idle_purr = delta_purr;
-
- physc = (delta_purr - delta_idle_purr) / delta_tb;
- idle = (delta_purr / delta_tb) - physc;
- idle *= 100.00;
+ /*
+ * Round down delta_purr to delta_tb if delta_tb - delta_purr
+ * error is under -1%.
+ */
+ if (((delta_tb - delta_purr + delta_idle_purr) / delta_tb * 100) > -1 && ((delta_tb - delta_purr + delta_idle_purr) / delta_tb * 100) < 0)
+ delta_purr = delta_tb;
+
+ if (!strcmp(mode, "Capped")) {
+ /* For dedicated - capped mode */
+ physc = (delta_purr - delta_idle_purr) / delta_tb;
+ idle = (delta_purr / delta_tb) - physc;
+ idle *= 100.00;
+ } else {
+ /* For shared and dedicated - donate mode */
+ idle = (delta_tb - delta_purr + delta_idle_purr) / delta_tb;
+ idle *= 100.00;
+ }

sprintf(buf, "%.2f", idle);
}
@@ -557,19 +576,35 @@ void get_cpu_util_spurr(struct sysentry *unused_se, char *buf)
void get_cpu_idle_spurr(struct sysentry *unused_se, char *buf)
{
double delta_tb, delta_spurr, delta_idle_spurr;
double physc, idle;
double rfreq;
+ char *descr;
+ char mode[32];

delta_tb = get_scaled_tb();
delta_spurr = get_delta_value("spurr");
delta_idle_spurr = get_delta_value("idle_spurr");

- physc = (delta_spurr - delta_idle_spurr) / delta_tb;
- idle = (delta_spurr / delta_tb) - physc;
- idle *= 100.00;
+ get_sysdata("shared_processor_mode", &descr, mode);
+ if (!strcmp(mode, "Dedicated"))
+ get_sysdata("DedDonMode", &descr, mode);
+
+ if (delta_spurr > delta_tb)
+ delta_spurr = delta_tb;
+
+ if (!strcmp(mode, "Capped")) {
+ /* For dedicated - capped mode */
+ physc = (delta_spurr - delta_idle_spurr) / delta_tb;
+ idle = (delta_spurr / delta_tb) - physc;
+ idle *= 100.00;
+ } else {
+ /* For shared and dedicated - donate mode */
+ idle = (delta_tb - delta_spurr + delta_idle_spurr) / delta_tb;
+ idle *= 100.00;
+ }

rfreq = round_off_freq();
idle += ((idle * rfreq) / 100);

sprintf(buf, "%.2f", idle);
}
--
2.39.3

[Tyrel Datwyler]

Patch applied to powerpc-utils/next branch.

https://github.com/ibm-power-utilities/powerpc-utils/commit/2af8c0b9a285e8a6104560d0f482819e56060443

Thanks,
-Tyrel

[Tyrel Datwyler]

On 7/17/24 12:09, Saket Kumar Bhaskar wrote:

> lparstat -E gives %busy and %idle for actual(PURR based) and normalized
> (SPURR based).Idle and busy PURR/SPURR values are not adding upto 100%
> in case of dedicated-donate and shared partitions, with the present
> formula. Because of this, users might get a false impression of resource
> utilisation. This is expected because a core can be in either
> idle or busy state out of total of 100(core's shared resource can
> either be consumed or be left idle). When lpar is in dedicated-donate
> or shared,the purr values are not being counted when the CPU is ceded.
> The idle_purr is calculated by taking snapshots of purr values at
> every idle entry and idle exit. So, when a CPU is ceded, the calculation
> for idle_purr will be wrong as purr is not being counted.
>

Saket,

The following github issue was raised, and I wonder if you might take a look
seeing as you authored this patch.

https://github.com/ibm-power-utilities/powerpc-utils/issues/103

-Tyrel

[Srikar Dronamraju]

* Tyrel Datwyler <tyr...@linux.ibm.com> [2024-11-22 14:54:26]:

I think this is because of the comment
https://groups.google.com/g/powerpc-utils-devel/c/IZs0Norg8nc/m/36FdzS0wAQAJ

Basically if delta_purr > delta_tb, comment was not to round-off since we
dont want to hide purr accounting errors.

Saket, what we could do is even if we show the delta_purr to be higher, we
could avoid idle to be zero, or +ve value.

>
> -Tyrel
>

--
Thanks and Regards
Srikar Dronamraju

[Saket Kumar Bhaskar]

Hi Tyrel,

As Srikar has pointed, I will look at this and send a patch for this.

Thanks,
Saket