<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 29-Oct-2021, at 11:46 AM, Nicholas Piggin <<a href="mailto:npiggin@gmail.com" class="">npiggin@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta charset="UTF-8" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Excerpts from Athira Rajeev's message of October 29, 2021 1:05 pm:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">During Live Partition Migration (LPM), it is observed that perf<br class="">counter values reports zero post migration completion. However<br class="">'perf stat' with workload continues to show counts post migration<br class="">since PMU gets disabled/enabled during sched switches. But incase<br class="">of system/cpu wide monitoring, zero counts were reported with 'perf<br class="">stat' after migration completion.<br class=""><br class="">Example:<br class="">./perf stat -e r1001e -I 1000<br class=""> time counts unit events<br class=""> 1.001010437 22,137,414 r1001e<br class=""> 2.002495447 15,455,821 r1001e<br class=""><<>> As seen in next below logs, the counter values shows zero<br class=""> after migration is completed.<br class=""><<>><br class=""> 86.142535370 129,392,333,440 r1001e<br class=""> 87.144714617 0 r1001e<br class=""> 88.146526636 0 r1001e<br class=""> 89.148085029 0 r1001e<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">This is the output without the patch? After the patch it keeps counting<span class="Apple-converted-space"> </span></span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">I suppose? And does the very large count go away too?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div>Hi Nick,</div><div><br class=""></div><div>Thanks for the review comments.</div><div>The output is without the patch. After the patch it keeps counting and negative count goes away.</div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">My bad, missed to keep both results in present version. </span>I will add the before and after patch results in next version.</div><div><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="">Here PMU is enabled during start of perf session and counter<br class="">values are read at intervals. Counters are only disabled at the<br class="">end of session. The powerpc mobility code presently does not handle<br class="">disabling and enabling back of PMU counters during partition<br class="">migration. Also since the PMU register values are not saved/restored<br class="">during migration, PMU registers like Monitor Mode Control Register 0<br class="">(MMCR0), Monitor Mode Control Register 1 (MMCR1) will not contain<br class="">the value it was programmed with. Hence PMU counters will not be<br class="">enabled correctly post migration.<br class=""><br class="">Fix this in mobility code by handling disabling and enabling of<br class="">PMU in all cpu's before and after migration. Patch introduces two<br class="">functions 'mobility_pmu_disable' and 'mobility_pmu_enable'.<br class="">mobility_pmu_disable() is called before the processor threads goes<br class="">to suspend state so as to disable the PMU counters. And disable is<br class="">done only if there are any active events running on that cpu.<br class="">mobility_pmu_enable() is called after the migrate is done to enable<br class="">back the PMU counters.<br class=""><br class="">Since the performance Monitor counters ( PMCs) are not<br class="">saved/restored during LPM, results in PMC value being zero and the<br class="">'event->hw.prev_count' being non-zero value. This causes problem<br class="">during updation of event->count since we always accumulate<br class="">(event->hw.prev_count - PMC value) in event->count. If<br class="">event->hw.prev_count is greater PMC value, event->count becomes<br class="">negative. To fix this, 'prev_count' also needs to be re-initialised<br class="">for all events while enabling back the events. Hence read the<br class="">existing events and clear the PMC index (stored in event->hw.idx)<br class="">for all events im mobility_pmu_disable. By this way, event count<br class="">settings will get re-initialised correctly in power_pmu_enable.<br class=""><br class="">Signed-off-by: Athira Rajeev <<a href="mailto:atrajeev@linux.vnet.ibm.com" class="">atrajeev@linux.vnet.ibm.com</a>><br class="">[ Fixed compilation error reported by kernel test robot ]<br class="">Reported-by: kernel test robot <<a href="mailto:lkp@intel.com" class="">lkp@intel.com</a>><br class="">---<br class="">Changelog:<br class="">Change from v2 -> v3:<br class="">Addressed review comments from Nicholas Piggin.<br class="">- Removed the "migrate" field which was added in initial<br class=""> patch to address updation of event count settings correctly<br class=""> in power_pmu_enable. Instead read off existing events in<br class=""> mobility_pmu_disable before power_pmu_enable.<br class="">- Moved the mobility_pmu_disable/enable declaration from<br class=""> rtas.h to perf event header file.<br class=""><br class="">Addressed review comments from Nathan.<br class="">- Moved the mobility function calls from stop_machine<br class=""> context out to pseries_migrate_partition. Also now this<br class=""> is a per cpu invocation.<br class=""><br class="">Change from v1 -> v2:<br class="">- Moved the mobility_pmu_enable and mobility_pmu_disable<br class=""> declarations under CONFIG_PPC_PERF_CTRS in rtas.h.<br class=""> Also included 'asm/rtas.h' in core-book3s to fix the<br class=""> compilation warning reported by kernel test robot.<br class=""><br class="">arch/powerpc/include/asm/perf_event.h | 8 +++++<br class="">arch/powerpc/perf/core-book3s.c | 39 +++++++++++++++++++++++<br class="">arch/powerpc/platforms/pseries/mobility.c | 7 ++++<br class="">3 files changed, 54 insertions(+)<br class=""><br class="">diff --git a/arch/powerpc/include/asm/perf_event.h b/arch/powerpc/include/asm/perf_event.h<br class="">index 164e910bf654..88aab6cf840c 100644<br class="">--- a/arch/powerpc/include/asm/perf_event.h<br class="">+++ b/arch/powerpc/include/asm/perf_event.h<br class="">@@ -17,6 +17,14 @@ static inline bool is_sier_available(void) { return false; }<br class="">static inline unsigned long get_pmcs_ext_regs(int idx) { return 0; }<br class="">#endif<br class=""><br class="">+#ifdef CONFIG_PPC_PERF_CTRS<br class="">+void mobility_pmu_disable(void *unused);<br class="">+void mobility_pmu_enable(void *unused);<br class="">+#else<br class="">+static inline void mobility_pmu_disable(void *unused) { }<br class="">+static inline void mobility_pmu_enable(void *unused) { }<br class="">+#endif<br class="">+<br class="">#ifdef CONFIG_FSL_EMB_PERF_EVENT<br class="">#include <asm/perf_event_fsl_emb.h><br class="">#endif<br class="">diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c<br class="">index 73e62e9b179b..2e8c4c668fa3 100644<br class="">--- a/arch/powerpc/perf/core-book3s.c<br class="">+++ b/arch/powerpc/perf/core-book3s.c<br class="">@@ -1343,6 +1343,33 @@ static void power_pmu_disable(struct pmu *pmu)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>local_irq_restore(flags);<br class="">}<br class=""><br class="">+/*<br class="">+ * Called from pseries_migrate_partition() function<br class="">+ * before migration, from powerpc/mobility code.<br class="">+ */<br class="">+void mobility_pmu_disable(void *unused)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>struct cpu_hw_events *cpuhw = this_cpu_ptr(&cpu_hw_events);<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>struct perf_event *event;<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>if (cpuhw->n_events != 0) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>int i;<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>power_pmu_disable(NULL);<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>/*<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>* Read off any pre-existing events because the register<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>* values may not be migrated.<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>*/<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>for (i = 0; i < cpuhw->n_events; ++i) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>event = cpuhw->event[i];<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>if (event->hw.idx) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>power_pmu_read(event);<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>event->hw.idx = 0;<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>}<br class="">+}<br class="">+<br class="">/*<br class=""> * Re-enable all events if disable == 0.<br class=""> * If we were previously disabled and events were added, then<br class="">@@ -1515,6 +1542,18 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>local_irq_restore(flags);<br class="">}<br class=""><br class="">+/*<br class="">+ * Called from pseries_migrate_partition() function<br class="">+ * after migration, from powerpc/mobility code.<br class="">+ */<br class="">+void mobility_pmu_enable(void *unused)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>struct cpu_hw_events *cpuhw = this_cpu_ptr(&cpu_hw_events);<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>cpuhw->n_added = cpuhw->n_events;<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>power_pmu_enable(NULL);<br class="">+}<br class="">+<br class="">static int collect_events(struct perf_event *group, int max_count,<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span> struct perf_event *ctrs[], u64 *events,<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span> unsigned int *flags)<br class="">diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c<br class="">index e83e0891272d..3e96485ccba4 100644<br class="">--- a/arch/powerpc/platforms/pseries/mobility.c<br class="">+++ b/arch/powerpc/platforms/pseries/mobility.c<br class="">@@ -22,6 +22,7 @@<br class="">#include <linux/delay.h><br class="">#include <linux/slab.h><br class="">#include <linux/stringify.h><br class="">+#include <linux/perf_event.h><br class=""><br class="">#include <asm/machdep.h><br class="">#include <asm/rtas.h><br class="">@@ -631,12 +632,18 @@ static int pseries_migrate_partition(u64 handle)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>if (ret)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>return ret;<br class=""><br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>/* Disable PMU before suspend */<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>on_each_cpu(&mobility_pmu_disable, NULL, 0);<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Why was this moved out of stop machine and to an IPI?</span></div></blockquote><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">My concern would be, what are the other CPUs doing at this time? Is it<span class="Apple-converted-space"> </span></span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">possible they could take interrupts and schedule? Could that mess up the</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">perf state here?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Thanks,</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Nick</span></div></blockquote></div><br class=""></body></html>