* [PATCH v2 1/2] powerpc/pseries: Add pool idle time at LPAR boot
2024-04-12 9:20 [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
@ 2024-04-12 9:20 ` Shrikanth Hegde
2024-04-12 9:20 ` [PATCH v2 2/2] powerpc/pseries: Add failure related checks for h_get_mpp and h_get_ppp Shrikanth Hegde
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Shrikanth Hegde @ 2024-04-12 9:20 UTC (permalink / raw
To: mpe; +Cc: nathanl, tyreld, sshegde, npiggin, mahesh, naveen.n.rao,
linuxppc-dev
When there are no options specified for lparstat, it is expected to
give reports since LPAR(Logical Partition) boot.
APP(Available Processor Pool) is an indicator of how many cores in the
shared pool are free to use in Shared Processor LPAR(SPLPAR). APP is
derived using pool_idle_time which is obtained using H_PIC call.
The interval based reports show correct APP value while since boot
report shows very high APP values. This happens because in that case APP
is obtained by dividing pool idle time by LPAR uptime. Since pool idle
time is reported by the PowerVM hypervisor since its boot, it need not
align with LPAR boot.
To fix that export boot pool idle time in lparcfg and powerpc-utils will
use this info to derive APP as below for since boot reports.
APP = (pool idle time - boot pool idle time) / (uptime * timebase)
Results:: Observe APP values.
====================== Shared LPAR ================================
lparstat
System Configuration
type=Shared mode=Uncapped smt=8 lcpu=12 mem=15573440 kB cpus=37 ent=12.00
reboot
stress-ng --cpu=$(nproc) -t 600
sleep 600
So in this case app is expected to close to 37-6=31.
====== 6.9-rc1 and lparstat 1.3.10 =============
%user %sys %wait %idle physc %entc lbusy app vcsw phint
----- ----- ----- ----- ----- ----- ----- ----- ----- -----
47.48 0.01 0.00 52.51 0.00 0.00 47.49 69099.72 541547 21
=== With this patch and powerpc-utils patch to do the above equation ===
%user %sys %wait %idle physc %entc lbusy app vcsw phint
----- ----- ----- ----- ----- ----- ----- ----- ----- -----
47.48 0.01 0.00 52.51 5.73 47.75 47.49 31.21 541753 21
=====================================================================
Note: physc, purr/idle purr being inaccurate is being handled in a
separate patch in powerpc-utils tree.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
arch/powerpc/platforms/pseries/lparcfg.c | 39 ++++++++++++++++++------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index f73c4d1c26af..5c2a3e802a02 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -170,20 +170,24 @@ static void show_gpci_data(struct seq_file *m)
kfree(buf);
}
-static unsigned h_pic(unsigned long *pool_idle_time,
- unsigned long *num_procs)
+static long h_pic(unsigned long *pool_idle_time,
+ unsigned long *num_procs)
{
- unsigned long rc;
- unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
+ long rc;
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
rc = plpar_hcall(H_PIC, retbuf);
- *pool_idle_time = retbuf[0];
- *num_procs = retbuf[1];
+ if (pool_idle_time)
+ *pool_idle_time = retbuf[0];
+ if (num_procs)
+ *num_procs = retbuf[1];
return rc;
}
+unsigned long boot_pool_idle_time;
+
/*
* parse_ppp_data
* Parse out the data returned from h_get_ppp and h_pic
@@ -215,9 +219,15 @@ static void parse_ppp_data(struct seq_file *m)
seq_printf(m, "pool_capacity=%d\n",
ppp_data.active_procs_in_pool * 100);
- h_pic(&pool_idle_time, &pool_procs);
- seq_printf(m, "pool_idle_time=%ld\n", pool_idle_time);
- seq_printf(m, "pool_num_procs=%ld\n", pool_procs);
+ /* In case h_pic call is not successful, this would result in
+ * APP values being wrong in tools like lparstat.
+ */
+
+ if (h_pic(&pool_idle_time, &pool_procs) == H_SUCCESS) {
+ seq_printf(m, "pool_idle_time=%ld\n", pool_idle_time);
+ seq_printf(m, "pool_num_procs=%ld\n", pool_procs);
+ seq_printf(m, "boot_pool_idle_time=%ld\n", boot_pool_idle_time);
+ }
}
seq_printf(m, "unallocated_capacity_weight=%d\n",
@@ -792,6 +802,7 @@ static const struct proc_ops lparcfg_proc_ops = {
static int __init lparcfg_init(void)
{
umode_t mode = 0444;
+ long retval;
/* Allow writing if we have FW_FEATURE_SPLPAR */
if (firmware_has_feature(FW_FEATURE_SPLPAR))
@@ -801,6 +812,16 @@ static int __init lparcfg_init(void)
printk(KERN_ERR "Failed to create powerpc/lparcfg\n");
return -EIO;
}
+
+ /* If this call fails, it would result in APP values
+ * being wrong for since boot reports of lparstat
+ */
+ retval = h_pic(&boot_pool_idle_time, NULL);
+
+ if (retval != H_SUCCESS)
+ pr_debug("H_PIC failed during lparcfg init retval: %ld\n",
+ retval);
+
return 0;
}
machine_device_initcall(pseries, lparcfg_init);
--
2.39.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] powerpc/pseries: Add failure related checks for h_get_mpp and h_get_ppp
2024-04-12 9:20 [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
2024-04-12 9:20 ` [PATCH v2 1/2] powerpc/pseries: Add pool idle time at LPAR boot Shrikanth Hegde
@ 2024-04-12 9:20 ` Shrikanth Hegde
2024-04-29 5:55 ` [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
2024-05-03 10:41 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Shrikanth Hegde @ 2024-04-12 9:20 UTC (permalink / raw
To: mpe; +Cc: nathanl, tyreld, sshegde, npiggin, mahesh, naveen.n.rao,
linuxppc-dev
Couple of Minor fixes:
- hcall return values are long. Fix that for h_get_mpp, h_get_ppp and
parse_ppp_data
- If hcall fails, values set should be at-least zero. It shouldn't be
uninitialized values. Fix that for h_get_mpp and h_get_ppp
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/platforms/pseries/lpar.c | 6 +++---
arch/powerpc/platforms/pseries/lparcfg.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index a41e542ba94d..3d642139b900 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -570,7 +570,7 @@ struct hvcall_mpp_data {
unsigned long backing_mem;
};
-int h_get_mpp(struct hvcall_mpp_data *);
+long h_get_mpp(struct hvcall_mpp_data *mpp_data);
struct hvcall_mpp_x_data {
unsigned long coalesced_bytes;
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 4e9916bb03d7..c1d8bee8f701 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -1886,10 +1886,10 @@ notrace void __trace_hcall_exit(long opcode, long retval, unsigned long *retbuf)
* h_get_mpp
* H_GET_MPP hcall returns info in 7 parms
*/
-int h_get_mpp(struct hvcall_mpp_data *mpp_data)
+long h_get_mpp(struct hvcall_mpp_data *mpp_data)
{
- int rc;
- unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
+ long rc;
rc = plpar_hcall9(H_GET_MPP, retbuf);
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index 5c2a3e802a02..ed2176d8a866 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -113,8 +113,8 @@ struct hvcall_ppp_data {
*/
static unsigned int h_get_ppp(struct hvcall_ppp_data *ppp_data)
{
- unsigned long rc;
- unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
+ long rc;
rc = plpar_hcall9(H_GET_PPP, retbuf);
@@ -197,7 +197,7 @@ static void parse_ppp_data(struct seq_file *m)
struct hvcall_ppp_data ppp_data;
struct device_node *root;
const __be32 *perf_level;
- int rc;
+ long rc;
rc = h_get_ppp(&ppp_data);
if (rc)
--
2.39.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports
2024-04-12 9:20 [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
2024-04-12 9:20 ` [PATCH v2 1/2] powerpc/pseries: Add pool idle time at LPAR boot Shrikanth Hegde
2024-04-12 9:20 ` [PATCH v2 2/2] powerpc/pseries: Add failure related checks for h_get_mpp and h_get_ppp Shrikanth Hegde
@ 2024-04-29 5:55 ` Shrikanth Hegde
2024-04-30 3:54 ` Michael Ellerman
2024-05-03 10:41 ` Michael Ellerman
3 siblings, 1 reply; 6+ messages in thread
From: Shrikanth Hegde @ 2024-04-29 5:55 UTC (permalink / raw
To: mpe, nathanl; +Cc: naveen.n.rao, tyreld, linuxppc-dev, mahesh, npiggin
On 4/12/24 2:50 PM, Shrikanth Hegde wrote:
> Currently lparstat reports which shows since LPAR boot are wrong for
> some fields. There is a need for storing the PIC(Pool Idle Count) at
> boot for accurate reporting. PATCH 1 Does that.
>
> While there, it was noticed that hcall return value is long and both
> h_get_ppp and h_get_mpp could set the uninitialized values if the hcall
> fails. PATCH 2 does that.
>
> v1 -> v2:
> - Nathan pointed out the issues surrounding the h_pic call. Addressed
> those.
> - Added a pr_debug if h_pic fails during lparcfg_init
> - If h_pic fails while reading lparcfg, related files are not exported.
> - Added failure checks for h_get_mpp, h_get_ppp calls as well.
>
> v1: https://lore.kernel.org/all/20240405101340.149171-1-sshegde@linux.ibm.com/
>
> Shrikanth Hegde (2):
> powerpc/pseries: Add pool idle time at LPAR boot
> powerpc/pseries: Add fail related checks for h_get_mpp and h_get_ppp
>
> arch/powerpc/include/asm/hvcall.h | 2 +-
> arch/powerpc/platforms/pseries/lpar.c | 6 ++--
> arch/powerpc/platforms/pseries/lparcfg.c | 45 +++++++++++++++++-------
> 3 files changed, 37 insertions(+), 16 deletions(-)
>
> --
> 2.39.3
>
Ping.
Michael, Nathan, Naveen
Any comments on these patches?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports
2024-04-29 5:55 ` [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
@ 2024-04-30 3:54 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2024-04-30 3:54 UTC (permalink / raw
To: Shrikanth Hegde, nathanl
Cc: naveen.n.rao, tyreld, linuxppc-dev, mahesh, npiggin
Shrikanth Hegde <sshegde@linux.ibm.com> writes:
> On 4/12/24 2:50 PM, Shrikanth Hegde wrote:
>> Currently lparstat reports which shows since LPAR boot are wrong for
>> some fields. There is a need for storing the PIC(Pool Idle Count) at
>> boot for accurate reporting. PATCH 1 Does that.
>>
>> While there, it was noticed that hcall return value is long and both
>> h_get_ppp and h_get_mpp could set the uninitialized values if the hcall
>> fails. PATCH 2 does that.
>>
>> v1 -> v2:
>> - Nathan pointed out the issues surrounding the h_pic call. Addressed
>> those.
>> - Added a pr_debug if h_pic fails during lparcfg_init
>> - If h_pic fails while reading lparcfg, related files are not exported.
>> - Added failure checks for h_get_mpp, h_get_ppp calls as well.
>>
>> v1: https://lore.kernel.org/all/20240405101340.149171-1-sshegde@linux.ibm.com/
>>
>> Shrikanth Hegde (2):
>> powerpc/pseries: Add pool idle time at LPAR boot
>> powerpc/pseries: Add fail related checks for h_get_mpp and h_get_ppp
>>
>> arch/powerpc/include/asm/hvcall.h | 2 +-
>> arch/powerpc/platforms/pseries/lpar.c | 6 ++--
>> arch/powerpc/platforms/pseries/lparcfg.c | 45 +++++++++++++++++-------
>> 3 files changed, 37 insertions(+), 16 deletions(-)
>>
>> --
>> 2.39.3
>>
>
> Ping.
>
> Michael, Nathan, Naveen
> Any comments on these patches?
Looks fine. I have it in next-test, will probably go into next tomorrow.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports
2024-04-12 9:20 [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
` (2 preceding siblings ...)
2024-04-29 5:55 ` [PATCH v2 0/2] powerpc/pseries: Fixes for lparstat boot reports Shrikanth Hegde
@ 2024-05-03 10:41 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2024-05-03 10:41 UTC (permalink / raw
To: mpe, Shrikanth Hegde
Cc: nathanl, tyreld, mahesh, npiggin, naveen.n.rao, linuxppc-dev
On Fri, 12 Apr 2024 14:50:45 +0530, Shrikanth Hegde wrote:
> Currently lparstat reports which shows since LPAR boot are wrong for
> some fields. There is a need for storing the PIC(Pool Idle Count) at
> boot for accurate reporting. PATCH 1 Does that.
>
> While there, it was noticed that hcall return value is long and both
> h_get_ppp and h_get_mpp could set the uninitialized values if the hcall
> fails. PATCH 2 does that.
>
> [...]
Applied to powerpc/next.
[1/2] powerpc/pseries: Add pool idle time at LPAR boot
https://git.kernel.org/powerpc/c/9c74ecfd0fc46e2eaf92c1b6169cc0c8a87f1dc2
[2/2] powerpc/pseries: Add failure related checks for h_get_mpp and h_get_ppp
https://git.kernel.org/powerpc/c/6d4341638516bf97b9a34947e0bd95035a8230a5
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread