* [PATCH v2 0/3] cpuidle-pseries: Parse extended CEDE information for idle.
@ 2020-07-29 6:47 Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 1/3] cpuidle-pseries: Set the latency-hint before entering CEDE Gautham R. Shenoy
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Gautham R. Shenoy @ 2020-07-29 6:47 UTC (permalink / raw)
To: Nicholas Piggin, Anton Blanchard, Nathan Lynch, Michael Ellerman,
Michael Neuling, Vaidyanathan Srinivasan
Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Hi,
This is a v2 of the patch series to parse the extended CEDE
information in the pseries-cpuidle driver.
The v1 of this patchset can be found here :
https://lore.kernel.org/linuxppc-dev/1594120299-31389-1-git-send-email-ego@linux.vnet.ibm.com/
The change from v1 --> v2 :
* Dropped Patches 4 and 5 which would expose extended idle-states,
that wakeup on external interrupts, to cpuidle framework. These
were RFC patches in v1. Dropped them because currently the only
extended CEDE state that wakesup on external interrupts is CEDE(1)
which adds no signifcant value over CEDE(0).
* Rebased the patches onto powerpc/merge.
* No changes in code for Patches 1-3.
Motivation:
===========
On pseries Dedicated Linux LPARs, apart from the polling snooze idle
state, we currently have the CEDE idle state which cedes the CPU to
the hypervisor with latency-hint = 0.
However, the PowerVM hypervisor supports additional extended CEDE
states, which can be queried through the "ibm,get-systems-parameter"
rtas-call with the CEDE_LATENCY_TOKEN. The hypervisor maps these
extended CEDE states to appropriate platform idle-states in order to
provide energy-savings as well as shifting power to the active
units. On existing pseries LPARs today we have extended CEDE with
latency-hints {1,2} supported.
The patches in this patchset, adds code to parse the CEDE latency
records provided by the hypervisor. We use this information to
determine the wakeup latency of the regular CEDE (which we have been
so far hardcoding to 10us while experimentally it is much lesser ~
1us), by looking at the wakeup latency provided by the hypervisor for
Extended CEDE states. Since the platform currently advertises Extended
CEDE 1 to have wakeup latency of 2us, we can be sure that the wakeup
latency of the regular CEDE is no more than this.
With Patches 1-3, we see an improvement in the single-threaded
performance on ebizzy.
2 ebizzy threads bound to the same big-core. 25% improvement in the
avg records/s (higher the better) with patches 1-3.
x without_patches
* with_patches
N Min Max Median Avg Stddev
x 10 2491089 5834307 5398375 4244335 1596244.9
* 10 2893813 5834474 5832448 5327281.3 1055941.4
We do not observe any major regression in either the context_switch2
benchmark or the schbench benchmark
context_switch2 across CPU0 CPU1 (Both belong to same big-core, but different
small cores). We observe a minor 0.14% regression in the number of
context-switches (higher is better).
x without_patch
* with_patch
N Min Max Median Avg Stddev
x 500 348872 362236 354712 354745.69 2711.827
* 500 349422 361452 353942 354215.4 2576.9258
context_switch2 across CPU0 CPU8 (Different big-cores). We observe a 0.37%
improvement in the number of context-switches (higher is better).
x without_patch
* with_patch
N Min Max Median Avg Stddev
x 500 287956 294940 288896 288977.23 646.59295
* 500 288300 294646 289582 290064.76 1161.9992
schbench:
No major difference could be seen until the 99.9th percentile.
Without-patch
Latency percentiles (usec)
50.0th: 29
75.0th: 39
90.0th: 49
95.0th: 59
*99.0th: 13104
99.5th: 14672
99.9th: 15824
min=0, max=17993
With-patch:
Latency percentiles (usec)
50.0th: 29
75.0th: 40
90.0th: 50
95.0th: 61
*99.0th: 13648
99.5th: 14768
99.9th: 15664
min=0, max=29812
Gautham R. Shenoy (3):
cpuidle-pseries: Set the latency-hint before entering CEDE
cpuidle-pseries: Add function to parse extended CEDE records
cpuidle-pseries : Fixup exit latency for CEDE(0)
drivers/cpuidle/cpuidle-pseries.c | 167 +++++++++++++++++++++++++++++++++++++-
1 file changed, 165 insertions(+), 2 deletions(-)
--
1.9.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/3] cpuidle-pseries: Set the latency-hint before entering CEDE
2020-07-29 6:47 [PATCH v2 0/3] cpuidle-pseries: Parse extended CEDE information for idle Gautham R. Shenoy
@ 2020-07-29 6:47 ` Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 2/3] cpuidle-pseries: Add function to parse extended CEDE records Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 3/3] cpuidle-pseries : Fixup exit latency for CEDE(0) Gautham R. Shenoy
2 siblings, 0 replies; 4+ messages in thread
From: Gautham R. Shenoy @ 2020-07-29 6:47 UTC (permalink / raw)
To: Nicholas Piggin, Anton Blanchard, Nathan Lynch, Michael Ellerman,
Michael Neuling, Vaidyanathan Srinivasan
Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
As per the PAPR, each H_CEDE call is associated with a latency-hint to
be passed in the VPA field "cede_latency_hint". The CEDE states that
we were implicitly entering so far is CEDE with latency-hint = 0.
This patch explicitly sets the latency hint corresponding to the CEDE
state that we are currently entering. While at it, we save the
previous hint, to be restored once we wakeup from CEDE. This will be
required in the future when we expose extended-cede states through the
cpuidle framework, where each of them will have a different
cede-latency hint.
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-pseries.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 3e058ad2..88e71c3 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -86,19 +86,27 @@ static void check_and_cede_processor(void)
}
}
+#define NR_CEDE_STATES 1 /* CEDE with latency-hint 0 */
+#define NR_DEDICATED_STATES (NR_CEDE_STATES + 1) /* Includes snooze */
+
+u8 cede_latency_hint[NR_DEDICATED_STATES];
static int dedicated_cede_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
+ u8 old_latency_hint;
pseries_idle_prolog();
get_lppaca()->donate_dedicated_cpu = 1;
+ old_latency_hint = get_lppaca()->cede_latency_hint;
+ get_lppaca()->cede_latency_hint = cede_latency_hint[index];
HMT_medium();
check_and_cede_processor();
local_irq_disable();
get_lppaca()->donate_dedicated_cpu = 0;
+ get_lppaca()->cede_latency_hint = old_latency_hint;
pseries_idle_epilog();
@@ -130,7 +138,7 @@ static int shared_cede_loop(struct cpuidle_device *dev,
/*
* States for dedicated partition case.
*/
-static struct cpuidle_state dedicated_states[] = {
+static struct cpuidle_state dedicated_states[NR_DEDICATED_STATES] = {
{ /* Snooze */
.name = "snooze",
.desc = "snooze",
--
1.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/3] cpuidle-pseries: Add function to parse extended CEDE records
2020-07-29 6:47 [PATCH v2 0/3] cpuidle-pseries: Parse extended CEDE information for idle Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 1/3] cpuidle-pseries: Set the latency-hint before entering CEDE Gautham R. Shenoy
@ 2020-07-29 6:47 ` Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 3/3] cpuidle-pseries : Fixup exit latency for CEDE(0) Gautham R. Shenoy
2 siblings, 0 replies; 4+ messages in thread
From: Gautham R. Shenoy @ 2020-07-29 6:47 UTC (permalink / raw)
To: Nicholas Piggin, Anton Blanchard, Nathan Lynch, Michael Ellerman,
Michael Neuling, Vaidyanathan Srinivasan
Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Currently we use CEDE with latency-hint 0 as the only other idle state
on a dedicated LPAR apart from the polling "snooze" state.
The platform might support additional extended CEDE idle states, which
can be discovered through the "ibm,get-system-parameter" rtas-call
made with CEDE_LATENCY_TOKEN.
This patch adds a function to obtain information about the extended
CEDE idle states from the platform and parse the contents to populate
an array of extended CEDE states. These idle states thus discovered
will be added to the cpuidle framework in the next patch.
dmesg on a POWER9 LPAR, demonstrating the output of parsing the
extended CEDE latency parameters.
[ 5.913180] xcede : xcede_record_size = 10
[ 5.913183] xcede : Record 0 : hint = 1, latency =0x400 tb-ticks, Wake-on-irq = 1
[ 5.913188] xcede : Record 1 : hint = 2, latency =0x3e8000 tb-ticks, Wake-on-irq = 0
[ 5.913193] cpuidle : Skipping the 2 Extended CEDE idle states
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-pseries.c | 129 +++++++++++++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 88e71c3..b1dc24d 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -21,6 +21,7 @@
#include <asm/runlatch.h>
#include <asm/idle.h>
#include <asm/plpar_wrappers.h>
+#include <asm/rtas.h>
static struct cpuidle_driver pseries_idle_driver = {
.name = "pseries_idle",
@@ -86,9 +87,120 @@ static void check_and_cede_processor(void)
}
}
-#define NR_CEDE_STATES 1 /* CEDE with latency-hint 0 */
+struct xcede_latency_records {
+ u8 latency_hint;
+ u64 wakeup_latency_tb_ticks;
+ u8 responsive_to_irqs;
+};
+
+/*
+ * XCEDE : Extended CEDE states discovered through the
+ * "ibm,get-systems-parameter" rtas-call with the token
+ * CEDE_LATENCY_TOKEN
+ */
+#define MAX_XCEDE_STATES 4
+#define XCEDE_LATENCY_RECORD_SIZE 10
+#define XCEDE_LATENCY_PARAM_MAX_LENGTH (2 + 2 + \
+ (MAX_XCEDE_STATES * XCEDE_LATENCY_RECORD_SIZE))
+
+#define CEDE_LATENCY_TOKEN 45
+
+#define NR_CEDE_STATES (MAX_XCEDE_STATES + 1) /* CEDE with latency-hint 0 */
#define NR_DEDICATED_STATES (NR_CEDE_STATES + 1) /* Includes snooze */
+struct xcede_latency_records xcede_records[MAX_XCEDE_STATES];
+unsigned int nr_xcede_records;
+char xcede_parameters[XCEDE_LATENCY_PARAM_MAX_LENGTH];
+
+static int parse_cede_parameters(void)
+{
+ int ret = -1, i;
+ u16 payload_length;
+ u8 xcede_record_size;
+ u32 total_xcede_records_size;
+ char *payload;
+
+ memset(xcede_parameters, 0, XCEDE_LATENCY_PARAM_MAX_LENGTH);
+
+ ret = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+ NULL, CEDE_LATENCY_TOKEN, __pa(xcede_parameters),
+ XCEDE_LATENCY_PARAM_MAX_LENGTH);
+
+ if (ret) {
+ pr_err("xcede: Error parsing CEDE_LATENCY_TOKEN\n");
+ return ret;
+ }
+
+ payload_length = be16_to_cpu(*(__be16 *)(&xcede_parameters[0]));
+ payload = &xcede_parameters[2];
+
+ /*
+ * If the platform supports the cede latency settings
+ * information system parameter it must provide the following
+ * information in the NULL terminated parameter string:
+ *
+ * a. The first byte is the length “N” of each cede
+ * latency setting record minus one (zero indicates a length
+ * of 1 byte).
+ *
+ * b. For each supported cede latency setting a cede latency
+ * setting record consisting of the first “N” bytes as per
+ * the following table.
+ *
+ * -----------------------------
+ * | Field | Field |
+ * | Name | Length |
+ * -----------------------------
+ * | Cede Latency | 1 Byte |
+ * | Specifier Value | |
+ * -----------------------------
+ * | Maximum wakeup | |
+ * | latency in | 8 Bytes|
+ * | tb-ticks | |
+ * -----------------------------
+ * | Responsive to | |
+ * | external | 1 Byte |
+ * | interrupts | |
+ * -----------------------------
+ *
+ * This version has cede latency record size = 10.
+ */
+ xcede_record_size = (u8)payload[0] + 1;
+
+ if (xcede_record_size != XCEDE_LATENCY_RECORD_SIZE) {
+ pr_err("xcede : Expected record-size %d. Observed size %d.\n",
+ XCEDE_LATENCY_RECORD_SIZE, xcede_record_size);
+ return -EINVAL;
+ }
+
+ pr_info("xcede : xcede_record_size = %d\n", xcede_record_size);
+
+ /*
+ * Since the payload_length includes the last NULL byte and
+ * the xcede_record_size, the remaining bytes correspond to
+ * array of all cede_latency settings.
+ */
+ total_xcede_records_size = payload_length - 2;
+ nr_xcede_records = total_xcede_records_size / xcede_record_size;
+
+ payload++;
+ for (i = 0; i < nr_xcede_records; i++) {
+ struct xcede_latency_records *record = &xcede_records[i];
+
+ record->latency_hint = (u8)payload[0];
+ record->wakeup_latency_tb_ticks =
+ be64_to_cpu(*(__be64 *)(&payload[1]));
+ record->responsive_to_irqs = (u8)payload[9];
+ payload += xcede_record_size;
+ pr_info("xcede : Record %d : hint = %u, latency =0x%llx tb-ticks, Wake-on-irq = %u\n",
+ i, record->latency_hint,
+ record->wakeup_latency_tb_ticks,
+ record->responsive_to_irqs);
+ }
+
+ return 0;
+}
+
u8 cede_latency_hint[NR_DEDICATED_STATES];
static int dedicated_cede_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
@@ -219,6 +331,19 @@ static int pseries_cpuidle_driver_init(void)
return 0;
}
+static int add_pseries_idle_states(void)
+{
+ int nr_states = 2; /* By default we have snooze, CEDE */
+
+ if (parse_cede_parameters())
+ return nr_states;
+
+ pr_info("cpuidle : Skipping the %d Extended CEDE idle states\n",
+ nr_xcede_records);
+
+ return nr_states;
+}
+
/*
* pseries_idle_probe()
* Choose state table for shared versus dedicated partition
@@ -241,7 +366,7 @@ static int pseries_idle_probe(void)
max_idle_state = ARRAY_SIZE(shared_states);
} else {
cpuidle_state_table = dedicated_states;
- max_idle_state = ARRAY_SIZE(dedicated_states);
+ max_idle_state = add_pseries_idle_states();
}
} else
return -ENODEV;
--
1.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 3/3] cpuidle-pseries : Fixup exit latency for CEDE(0)
2020-07-29 6:47 [PATCH v2 0/3] cpuidle-pseries: Parse extended CEDE information for idle Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 1/3] cpuidle-pseries: Set the latency-hint before entering CEDE Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 2/3] cpuidle-pseries: Add function to parse extended CEDE records Gautham R. Shenoy
@ 2020-07-29 6:47 ` Gautham R. Shenoy
2 siblings, 0 replies; 4+ messages in thread
From: Gautham R. Shenoy @ 2020-07-29 6:47 UTC (permalink / raw)
To: Nicholas Piggin, Anton Blanchard, Nathan Lynch, Michael Ellerman,
Michael Neuling, Vaidyanathan Srinivasan
Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
We are currently assuming that CEDE(0) has exit latency 10us, since
there is no way for us to query from the platform. However, if the
wakeup latency of an Extended CEDE state is smaller than 10us, then we
can be sure that the exit latency of CEDE(0) cannot be more than that.
that.
In this patch, we fix the exit latency of CEDE(0) if we discover an
Extended CEDE state with wakeup latency smaller than 10us.
Benchmark results:
ebizzy:
2 ebizzy threads bound to the same big-core. 25% improvement in the
avg records/s with patch.
x without_patch
+ with_patch
N Min Max Median Avg Stddev
x 10 2491089 5834307 5398375 4244335 1596244.9
+ 10 2893813 5834474 5832448 5327281.3 1055941.4
context_switch2 :
There is no major regression observed with this patch as seen from the
context_switch2 benchmark.
context_switch2 across CPU0 CPU1 (Both belong to same big-core, but different
small cores). We observe a minor 0.14% regression in the number of
context-switches (higher is better).
x without_patch
+ with_patch
N Min Max Median Avg Stddev
x 500 348872 362236 354712 354745.69 2711.827
+ 500 349422 361452 353942 354215.4 2576.9258
Difference at 99.0% confidence
-530.288 +/- 430.963
-0.149484% +/- 0.121485%
(Student's t, pooled s = 2645.24)
context_switch2 across CPU0 CPU8 (Different big-cores). We observe a 0.37%
improvement in the number of context-switches (higher is better).
x without_patch
+ with_patch
N Min Max Median Avg Stddev
x 500 287956 294940 288896 288977.23 646.59295
+ 500 288300 294646 289582 290064.76 1161.9992
Difference at 99.0% confidence
1087.53 +/- 153.194
0.376337% +/- 0.0530125%
(Student's t, pooled s = 940.299)
schbench:
No major difference could be seen until the 99.9th percentile.
Without-patch
Latency percentiles (usec)
50.0th: 29
75.0th: 39
90.0th: 49
95.0th: 59
*99.0th: 13104
99.5th: 14672
99.9th: 15824
min=0, max=17993
With-patch:
Latency percentiles (usec)
50.0th: 29
75.0th: 40
90.0th: 50
95.0th: 61
*99.0th: 13648
99.5th: 14768
99.9th: 15664
min=0, max=29812
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-pseries.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index b1dc24d..0b2f115 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -334,12 +334,42 @@ static int pseries_cpuidle_driver_init(void)
static int add_pseries_idle_states(void)
{
int nr_states = 2; /* By default we have snooze, CEDE */
+ int i;
+ u64 min_latency_us = dedicated_states[1].exit_latency; /* CEDE latency */
if (parse_cede_parameters())
return nr_states;
- pr_info("cpuidle : Skipping the %d Extended CEDE idle states\n",
- nr_xcede_records);
+ for (i = 0; i < nr_xcede_records; i++) {
+ u64 latency_tb = xcede_records[i].wakeup_latency_tb_ticks;
+ u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC;
+
+ if (latency_us < min_latency_us)
+ min_latency_us = latency_us;
+ }
+
+ /*
+ * We are currently assuming that CEDE(0) has exit latency
+ * 10us, since there is no way for us to query from the
+ * platform.
+ *
+ * However, if the wakeup latency of an Extended CEDE state is
+ * smaller than 10us, then we can be sure that CEDE(0)
+ * requires no more than that.
+ *
+ * Perform the fix-up.
+ */
+ if (min_latency_us < dedicated_states[1].exit_latency) {
+ u64 cede0_latency = min_latency_us - 1;
+
+ if (cede0_latency <= 0)
+ cede0_latency = min_latency_us;
+
+ dedicated_states[1].exit_latency = cede0_latency;
+ dedicated_states[1].target_residency = 10 * (cede0_latency);
+ pr_info("cpuidle : Fixed up CEDE exit latency to %llu us\n",
+ cede0_latency);
+ }
return nr_states;
}
--
1.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-29 6:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 6:47 [PATCH v2 0/3] cpuidle-pseries: Parse extended CEDE information for idle Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 1/3] cpuidle-pseries: Set the latency-hint before entering CEDE Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 2/3] cpuidle-pseries: Add function to parse extended CEDE records Gautham R. Shenoy
2020-07-29 6:47 ` [PATCH v2 3/3] cpuidle-pseries : Fixup exit latency for CEDE(0) Gautham R. Shenoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).