From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com> To: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> Cc: Nicholas Piggin <npiggin@gmail.com>, Anton Blanchard <anton@ozlabs.org>, Nathan Lynch <nathanl@linux.ibm.com>, Michael Ellerman <mpe@ellerman.id.au>, Michael Neuling <mikey@neuling.org>, linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org Subject: Re: [PATCH 3/5] cpuidle-pseries : Fixup exit latency for CEDE(0) Date: Mon, 20 Jul 2020 11:54:16 +0530 [thread overview] Message-ID: <20200720062416.GD4000@drishya.in.ibm.com> (raw) In-Reply-To: <1594120299-31389-4-git-send-email-ego@linux.vnet.ibm.com> * Gautham R Shenoy <ego@linux.vnet.ibm.com> [2020-07-07 16:41:37]: > 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. The new > value is 1us lesser than the smallest wakeup latency among the > Extended CEDE states. > > 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 > > 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 > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.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 c13549b..502f906 100644 > --- a/drivers/cpuidle/cpuidle-pseries.c > +++ b/drivers/cpuidle/cpuidle-pseries.c > @@ -353,12 +353,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); > + } As per PAPR spec the CEDE hints are in increasing order of exit latency. Hence a given state's exit latency cannot exceed the one following it. The quirk is such that the first one (hint 0) is implicit and hence we have to use the above logic to extract its characteristics. --Vaidy
WARNING: multiple messages have this Message-ID (diff)
From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com> To: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> Cc: Nathan Lynch <nathanl@linux.ibm.com>, Michael Neuling <mikey@neuling.org>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>, linuxppc-dev@ozlabs.org Subject: Re: [PATCH 3/5] cpuidle-pseries : Fixup exit latency for CEDE(0) Date: Mon, 20 Jul 2020 11:54:16 +0530 [thread overview] Message-ID: <20200720062416.GD4000@drishya.in.ibm.com> (raw) In-Reply-To: <1594120299-31389-4-git-send-email-ego@linux.vnet.ibm.com> * Gautham R Shenoy <ego@linux.vnet.ibm.com> [2020-07-07 16:41:37]: > 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. The new > value is 1us lesser than the smallest wakeup latency among the > Extended CEDE states. > > 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 > > 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 > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.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 c13549b..502f906 100644 > --- a/drivers/cpuidle/cpuidle-pseries.c > +++ b/drivers/cpuidle/cpuidle-pseries.c > @@ -353,12 +353,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); > + } As per PAPR spec the CEDE hints are in increasing order of exit latency. Hence a given state's exit latency cannot exceed the one following it. The quirk is such that the first one (hint 0) is implicit and hence we have to use the above logic to extract its characteristics. --Vaidy
next prev parent reply other threads:[~2020-07-20 6:24 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-07 11:11 [PATCH 0/5] cpuidle-pseries: Parse extended CEDE information for idle Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-07 11:11 ` [PATCH 1/5] cpuidle-pseries: Set the latency-hint before entering CEDE Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-20 5:55 ` Vaidyanathan Srinivasan 2020-07-20 5:55 ` Vaidyanathan Srinivasan 2020-07-07 11:11 ` [PATCH 2/5] cpuidle-pseries: Add function to parse extended CEDE records Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-20 6:09 ` Vaidyanathan Srinivasan 2020-07-20 6:09 ` Vaidyanathan Srinivasan 2020-07-07 11:11 ` [PATCH 3/5] cpuidle-pseries : Fixup exit latency for CEDE(0) Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-20 6:24 ` Vaidyanathan Srinivasan [this message] 2020-07-20 6:24 ` Vaidyanathan Srinivasan 2020-07-07 11:11 ` [PATCH 4/5] cpuidle-pseries : Include extended CEDE states in cpuidle framework Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-20 6:31 ` Vaidyanathan Srinivasan 2020-07-20 6:31 ` Vaidyanathan Srinivasan 2020-07-07 11:11 ` [PATCH 5/5] cpuidle-pseries: Block Extended CEDE(1) which adds no additional value Gautham R. Shenoy 2020-07-07 11:11 ` Gautham R. Shenoy 2020-07-20 6:34 ` Vaidyanathan Srinivasan 2020-07-20 6:34 ` Vaidyanathan Srinivasan 2020-07-07 11:32 ` [PATCH 0/5] cpuidle-pseries: Parse extended CEDE information for idle Gautham R Shenoy 2020-07-07 11:32 ` Gautham R Shenoy 2020-07-27 14:14 ` Rafael J. Wysocki 2020-07-27 14:14 ` Rafael J. Wysocki 2020-07-27 18:55 ` Gautham R Shenoy 2020-07-27 18:55 ` Gautham R Shenoy 2020-07-20 5:48 ` Vaidyanathan Srinivasan 2020-07-20 5:48 ` Vaidyanathan Srinivasan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200720062416.GD4000@drishya.in.ibm.com \ --to=svaidy@linux.ibm.com \ --cc=anton@ozlabs.org \ --cc=ego@linux.vnet.ibm.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linuxppc-dev@ozlabs.org \ --cc=mikey@neuling.org \ --cc=mpe@ellerman.id.au \ --cc=nathanl@linux.ibm.com \ --cc=npiggin@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.