[PATCH 1/2] lparstat: report mixed SMT state

[Laurent Dufour]

when SMT state is mixed like this one (CPU 4 is offline):

$ ppc64_cpu --info
Core 0: 0* 1* 2* 3* 4 5* 6* 7*
Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
Core 2: 16* 17* 18* 19* 20* 21* 22* 23*
Core 3: 24* 25* 26* 27* 28* 29* 30* 31*
Core 4: 32* 33* 34* 35* 36* 37* 38* 39*
Core 5: 40* 41* 42* 43* 44* 45* 46* 47*
$ ppc64_cpu --smt
SMT=7: 0
SMT=8: 1-5

ppc64_cpu --smt is handling that nicely but lparstat failed reporting the
SMT state:
$ /usr/sbin/lparstat
Failed to get smt state

System Configuration
type=Dedicated mode=Capped smt=Capped lcpu=6 mem=65969728 kB cpus=0 ent=6.00

%user %sys %wait %idle physc %entc lbusy app vcsw phint
----- ----- ----- ----- ----- ----- ----- ----- ----- -----
0.02 0.01 0.00 99.97 3.41 56.83 0.02 0.00 4061778 156

Makes lparstat reporting "smt=mixed" in that case.
__do_smt is now returning 0 when the SMT state is mixed instead of -1 which
is also reported when an error is detected.
This doesn't change the call made by ppc64_cpu which is using
print_smt_state=true and so is expecting a returned value equal to 0 or -1.

With that patch applied, lparstat print that in the above case:
$lparstat

System Configuration
type=Dedicated mode=Capped smt=Mixed lcpu=6 mem=65969728 kB cpus=0 ent=6.00

%user %sys %wait %idle physc %entc lbusy app vcsw phint
----- ----- ----- ----- ----- ----- ----- ----- ----- -----
0.01 0.01 0.00 99.97 3.43 57.17 0.02 0.00 4105654 156

Signed-off-by: Laurent Dufour <ldu...@linux.ibm.com>
---
src/common/cpu_info_helpers.c | 4 ++--
src/lparstat.c | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/common/cpu_info_helpers.c b/src/common/cpu_info_helpers.c
index 925f220210f6..c05d96d5b3cc 100644
--- a/src/common/cpu_info_helpers.c
+++ b/src/common/cpu_info_helpers.c
@@ -245,7 +245,7 @@ int __do_smt(bool numeric, int cpus_in_system, int threads_per_cpu,
if (smt_state == 0)
smt_state = thread + 1;
else if (smt_state > 0)
- smt_state = -1; /* mix of SMT modes */
+ smt_state = 0; /* mix of SMT modes */
}
}

@@ -257,7 +257,7 @@ int __do_smt(bool numeric, int cpus_in_system, int threads_per_cpu,
printf("SMT=1\n");
else
printf("SMT is off\n");
- } else if (smt_state == -1) {
+ } else if (smt_state == 0) {
for (thread = 0; thread < threads_per_cpu; thread++) {
if (CPU_COUNT_S(cpu_state_size,
cpu_states[thread])) {
diff --git a/src/lparstat.c b/src/lparstat.c
index eebba1fff9ab..a9e7bceefab3 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -884,13 +884,15 @@ void get_smt_mode(struct sysentry *se, char *buf)
}

smt_state = parse_smt_state();
- if (smt_state < 0) {
+ if (smt_state == -1) {
fprintf(stderr, "Failed to get smt state\n");
return;
}

if (smt_state == 1)
sprintf(buf, "Off");
+ else if (smt_state == 0)
+ sprintf(buf, "Mixed");
else
sprintf(buf, "%d", smt_state);
}
--
2.40.1

[Laurent Dufour]

When threads are offlined, lparstat is failing with the following error message:
$ lparstat -E
Failed to read /sys/devices/system/cpu/cpu0/spurr

In addition, when the SMT state is mixed, lparstat is confused and report
wrong information:

$ /usr/sbin/lparstat
Failed to get smt state

System Configuration
type=Dedicated mode=Capped smt=Capped lcpu=6 mem=65969728 kB cpus=0 ent=6.00

With this series, lparstat is reporting the mixed SMT state:

$lparstat

System Configuration
type=Dedicated mode=Capped smt=Mixed lcpu=6 mem=65969728 kB cpus=0 ent=6.00

Laurent Dufour (2):
lparstat: report mixed SMT state
lparstat: Fix offline threads uninitialized entries

src/common/cpu_info_helpers.c | 4 ++--
src/lparstat.c | 8 +++++++-
2 files changed, 9 insertions(+), 3 deletions(-)

--
2.40.1

[Laurent Dufour]

When some threads are offline, lparstat -E is failing like that:

$ ppc64_cpu --info # CPU 20 is offline
Core 0: 0* 1* 2* 3* 4* 5* 6* 7*

Core 1: 8* 9* 10* 11* 12* 13* 14* 15*

Core 2: 16* 17* 18* 19* 20 21* 22* 23*

Core 3: 24* 25* 26* 27* 28* 29* 30* 31*
Core 4: 32* 33* 34* 35* 36* 37* 38* 39*
Core 5: 40* 41* 42* 43* 44* 45* 46* 47*

$ lparstat -E
Failed to read /sys/devices/system/cpu/cpu0/spurr

The message is complaining about CPU0 but the real issue is that in
parse_sysfs_values() the test cpu_sysfs_fds[i].spurr >= 0 is valid even if
the entry has not been initialized (cpu_sysfs_fds is alloc cleared). So
if the number of threads online seen in assign_cpu_sysfs_fds is lower than
threads_in_system, the loop in parse_sysfs_values() will read uninitialized
entry, where .cpu=0.

To prevent that, unset entries in the cpu_sysfs_fds should have the spurr
fd set to -1.

Signed-off-by: Laurent Dufour <ldu...@linux.ibm.com>
---

src/lparstat.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/src/lparstat.c b/src/lparstat.c
index a9e7bceefab3..d2fdb3ffde50 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -163,6 +163,10 @@ static int assign_cpu_sysfs_fds(int threads_in_system)
cpu_idx++;
}

+ /* Mark extra slots for offline threads unset, see parse_sysfs_values */
+ for (; cpu_idx < threads_in_system; cpu_idx++)
+ cpu_sysfs_fds[cpu_idx].spurr = -1;
+
return 0;
error:
fprintf(stderr, "Failed to open %s: %s\n",
--
2.40.1

[Laurent Dufour]

Hi,

It's been more than a month and I have sent this series.
Nobody replied, does that means nobody has any objections, and it's all
good, right?

[Tyrel Datwyler]

On 6/6/23 09:53, Laurent Dufour wrote:
> Hi,

>
> It's been more than a month and I have sent this series.
> Nobody replied, does that means nobody has any objections, and it's all
> good, right?
>

Doesn't appear to be any complaints. I will look into getting it merged when I
have a minute.

-Tyrel

[Tyrel Datwyler]

Series applied to powerpc-utils/next branch.

[1/2]
https://github.com/ibm-power-utilities/powerpc-utils/commit/b2672fa3d462217ccd057a2cd307af2448e78757

[2/2]
https://github.com/ibm-power-utilities/powerpc-utils/commit/dee15756bcb287ccf39a904be07c90107b13844b

Thanks,
-Tyrel