From c339f9be304c21da1c42899a824f84a2cc9ced30 Mon Sep 17 00:00:00 2001 From: Abhishek Goel Date: Mon, 6 Jul 2020 00:32:58 -0500 Subject: cpuidle/powernv : Remove dead code block Commit 1961acad2f88559c2cdd2ef67c58c3627f1f6e54 removes usage of function "validate_dt_prop_sizes". This patch removes this unused function. Signed-off-by: Abhishek Goel Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20200706053258.121475-1-huntbag@linux.vnet.ibm.com --- drivers/cpuidle/cpuidle-powernv.c | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index 1b299e801f74..addaa6e6718b 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -244,20 +244,6 @@ static inline void add_powernv_state(int index, const char *name, stop_psscr_table[index].mask = psscr_mask; } -/* - * Returns 0 if prop1_len == prop2_len. Else returns -1 - */ -static inline int validate_dt_prop_sizes(const char *prop1, int prop1_len, - const char *prop2, int prop2_len) -{ - if (prop1_len == prop2_len) - return 0; - - pr_warn("cpuidle-powernv: array sizes don't match for %s and %s\n", - prop1, prop2); - return -1; -} - extern u32 pnv_get_supported_cpuidle_states(void); static int powernv_add_idle_states(void) { -- cgit v1.2.3 From 92fe8483b1660feaa602d8be6ca7efe95ae4789b Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 14 Jul 2020 22:24:24 +0800 Subject: cpuidle/pseries: Make symbol 'pseries_idle_driver' static The sparse tool complains as follows: drivers/cpuidle/cpuidle-pseries.c:25:23: warning: symbol 'pseries_idle_driver' was not declared. Should it be static? 'pseries_idle_driver' is not used outside of this file, so marks it static. Reported-by: Hulk Robot Signed-off-by: Wei Yongjun Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20200714142424.66648-1-weiyongjun1@huawei.com --- drivers/cpuidle/cpuidle-pseries.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c index 6513ef2af66a..3e058ad2bb51 100644 --- a/drivers/cpuidle/cpuidle-pseries.c +++ b/drivers/cpuidle/cpuidle-pseries.c @@ -22,7 +22,7 @@ #include #include -struct cpuidle_driver pseries_idle_driver = { +static struct cpuidle_driver pseries_idle_driver = { .name = "pseries_idle", .owner = THIS_MODULE, }; -- cgit v1.2.3 From 3af0ada7dd98c6da35c1fd7f107af3b9aa5e904c Mon Sep 17 00:00:00 2001 From: "Gautham R. Shenoy" Date: Thu, 30 Jul 2020 11:02:55 +0530 Subject: cpuidle: pseries: Set the latency-hint before entering CEDE 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. Signed-off-by: Gautham R. Shenoy [mpe: Make cede_latency_hint static] Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/1596087177-30329-2-git-send-email-ego@linux.vnet.ibm.com --- drivers/cpuidle/cpuidle-pseries.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c index 3e058ad2bb51..31c8e51162b2 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_DEDICATED_STATES 2 /* snooze, CEDE */ + +static 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", @@ -233,7 +241,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 = NR_DEDICATED_STATES; } } else return -ENODEV; -- cgit v1.2.3 From 054e44ba99ae36918631fcbf5f034e466c2f1b73 Mon Sep 17 00:00:00 2001 From: "Gautham R. Shenoy" Date: Thu, 30 Jul 2020 11:02:56 +0530 Subject: cpuidle: pseries: Add function to parse extended CEDE records 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 POWER8 and POWER9 LPAR, demonstrating the output of parsing the extended CEDE latency parameters are as follows POWER8 [ 10.093279] xcede : xcede_record_size = 10 [ 10.093285] xcede : Record 0 : hint = 1, latency = 0x3c00 tb ticks, Wake-on-irq = 1 [ 10.093291] xcede : Record 1 : hint = 2, latency = 0x4e2000 tb ticks, Wake-on-irq = 0 [ 10.093297] cpuidle : Skipping the 2 Extended CEDE idle states POWER9 [ 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 Signed-off-by: Gautham R. Shenoy [mpe: Make space for 16 records, drop memset, minor cleanup & formatting] Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/1596087177-30329-3-git-send-email-ego@linux.vnet.ibm.com --- drivers/cpuidle/cpuidle-pseries.c | 136 +++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c index 31c8e51162b2..abfc160c9837 100644 --- a/drivers/cpuidle/cpuidle-pseries.c +++ b/drivers/cpuidle/cpuidle-pseries.c @@ -21,6 +21,7 @@ #include #include #include +#include static struct cpuidle_driver pseries_idle_driver = { .name = "pseries_idle", @@ -86,8 +87,131 @@ static void check_and_cede_processor(void) } } -#define NR_DEDICATED_STATES 2 /* snooze, CEDE */ +/* + * XCEDE: Extended CEDE states discovered through the + * "ibm,get-systems-parameter" RTAS call with the token + * CEDE_LATENCY_TOKEN + */ + +/* + * Section 7.3.16 System Parameters Option of PAPR version 2.8.1 has a + * table with all the parameters to ibm,get-system-parameters. + * CEDE_LATENCY_TOKEN corresponds to the token value for Cede Latency + * Settings Information. + */ +#define CEDE_LATENCY_TOKEN 45 + +/* + * 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. + * + * The structure xcede_latency_payload represents a) and b) with + * xcede_latency_record representing the table in b). + * + * xcede_latency_parameter is what gets returned by + * ibm,get-systems-parameter RTAS call when made with + * CEDE_LATENCY_TOKEN. + * + * These structures are only used to represent the data obtained by the RTAS + * call. The data is in big-endian. + */ +struct xcede_latency_record { + u8 hint; + __be64 latency_ticks; + u8 wake_on_irqs; +} __packed; + +// Make space for 16 records, which "should be enough". +struct xcede_latency_payload { + u8 record_size; + struct xcede_latency_record records[16]; +} __packed; + +struct xcede_latency_parameter { + __be16 payload_size; + struct xcede_latency_payload payload; + u8 null_char; +} __packed; + +static unsigned int nr_xcede_records; +static struct xcede_latency_parameter xcede_latency_parameter __initdata; + +static int __init parse_cede_parameters(void) +{ + struct xcede_latency_payload *payload; + u32 total_xcede_records_size; + u8 xcede_record_size; + u16 payload_size; + int ret, i; + + ret = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1, + NULL, CEDE_LATENCY_TOKEN, __pa(&xcede_latency_parameter), + sizeof(xcede_latency_parameter)); + if (ret) { + pr_err("xcede: Error parsing CEDE_LATENCY_TOKEN\n"); + return ret; + } + + payload_size = be16_to_cpu(xcede_latency_parameter.payload_size); + payload = &xcede_latency_parameter.payload; + xcede_record_size = payload->record_size + 1; + + if (xcede_record_size != sizeof(struct xcede_latency_record)) { + pr_err("xcede: Expected record-size %lu. Observed size %u.\n", + sizeof(struct xcede_latency_record), xcede_record_size); + return -EINVAL; + } + + pr_info("xcede: xcede_record_size = %d\n", xcede_record_size); + + /* + * Since the payload_size 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_size - 2; + nr_xcede_records = total_xcede_records_size / xcede_record_size; + + for (i = 0; i < nr_xcede_records; i++) { + struct xcede_latency_record *record = &payload->records[i]; + u64 latency_ticks = be64_to_cpu(record->latency_ticks); + u8 wake_on_irqs = record->wake_on_irqs; + u8 hint = record->hint; + + pr_info("xcede: Record %d : hint = %u, latency = 0x%llx tb ticks, Wake-on-irq = %u\n", + i, hint, latency_ticks, wake_on_irqs); + } + + return 0; +} + +#define NR_DEDICATED_STATES 2 /* snooze, CEDE */ static u8 cede_latency_hint[NR_DEDICATED_STATES]; static int dedicated_cede_loop(struct cpuidle_device *dev, @@ -219,6 +343,15 @@ static int pseries_cpuidle_driver_init(void) return 0; } +static void __init parse_xcede_idle_states(void) +{ + if (parse_cede_parameters()) + return; + + pr_info("cpuidle : Skipping the %d Extended CEDE idle states\n", + nr_xcede_records); +} + /* * pseries_idle_probe() * Choose state table for shared versus dedicated partition @@ -240,6 +373,7 @@ static int pseries_idle_probe(void) cpuidle_state_table = shared_states; max_idle_state = ARRAY_SIZE(shared_states); } else { + parse_xcede_idle_states(); cpuidle_state_table = dedicated_states; max_idle_state = NR_DEDICATED_STATES; } -- cgit v1.2.3 From d947fb4c965cdb7242f3f91124ea16079c49fa8b Mon Sep 17 00:00:00 2001 From: "Gautham R. Shenoy" Date: Thu, 30 Jul 2020 11:02:57 +0530 Subject: cpuidle: pseries: Fixup exit latency for CEDE(0) 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. 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: On POWER8, this patch does not have any impact since the advertized latency of Extended CEDE (1) is 30us which is higher than the default latency of CEDE (0) which is 10us. On POWER9 we see improvement the single-threaded performance of ebizzy, and no regression in the wakeup latency or the number of context-switches. 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 Signed-off-by: Gautham R. Shenoy [mpe: Minor formatting] Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/1596087177-30329-4-git-send-email-ego@linux.vnet.ibm.com --- drivers/cpuidle/cpuidle-pseries.c | 45 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c index abfc160c9837..ff6d99e923a4 100644 --- a/drivers/cpuidle/cpuidle-pseries.c +++ b/drivers/cpuidle/cpuidle-pseries.c @@ -343,13 +343,52 @@ static int pseries_cpuidle_driver_init(void) return 0; } -static void __init parse_xcede_idle_states(void) +static void __init fixup_cede0_latency(void) { + struct xcede_latency_payload *payload; + u64 min_latency_us; + int i; + + min_latency_us = dedicated_states[1].exit_latency; // CEDE latency + if (parse_cede_parameters()) return; - pr_info("cpuidle : Skipping the %d Extended CEDE idle states\n", + pr_info("cpuidle: Skipping the %d Extended CEDE idle states\n", nr_xcede_records); + + payload = &xcede_latency_parameter.payload; + for (i = 0; i < nr_xcede_records; i++) { + struct xcede_latency_record *record = &payload->records[i]; + u64 latency_tb = be64_to_cpu(record->latency_ticks); + u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC; + + if (latency_us < min_latency_us) + min_latency_us = latency_us; + } + + /* + * By default, we assume 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); + } + } /* @@ -373,7 +412,7 @@ static int pseries_idle_probe(void) cpuidle_state_table = shared_states; max_idle_state = ARRAY_SIZE(shared_states); } else { - parse_xcede_idle_states(); + fixup_cede0_latency(); cpuidle_state_table = dedicated_states; max_idle_state = NR_DEDICATED_STATES; } -- cgit v1.2.3