<html><head></head><body dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"><div 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 04-Nov-2021, at 11:25 AM, Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" class="">mpe@ellerman.id.au</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="">Nathan Lynch <</span><a href="mailto:nathanl@linux.ibm.com" 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;" class="">nathanl@linux.ibm.com</a><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="">> writes:</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="">Nicholas Piggin <<a href="mailto:npiggin@gmail.com" class="">npiggin@gmail.com</a>> writes:<br class=""><blockquote type="cite" class="">Excerpts from Michael Ellerman's message of October 29, 2021 11:15 pm:<br class=""><blockquote type="cite" class="">Nicholas Piggin <<a href="mailto:npiggin@gmail.com" class="">npiggin@gmail.com</a>> writes:<br class=""><blockquote type="cite" class="">Excerpts from Athira Rajeev's message of October 29, 2021 1:05 pm:<br class=""><blockquote type="cite" 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 class="">Why was this moved out of stop machine and to an IPI?<br class=""><br class="">My concern would be, what are the other CPUs doing at this time? Is it<span class="Apple-converted-space"> </span><br class="">possible they could take interrupts and schedule? Could that mess up the<br class="">perf state here?<br class=""></blockquote><br class="">pseries_migrate_partition() is called directly from migration_store(),<br class="">which is the sysfs store function, which can be called concurrently by<br class="">different CPUs.<br class=""><br class="">It's also potentially called from rtas_syscall_dispatch_ibm_suspend_me(),<br class="">from sys_rtas(), again with no locking.<br class=""><br class="">So we could have two CPUs calling into here at the same time, which<br class="">might not crash, but is unlikely to work well.<br class=""><br class="">I think the lack of locking might have been OK in the past because only<br class="">one CPU will successfully get the other CPUs to call do_join() in<br class="">pseries_suspend(). But I could be wrong.<br class=""><br class="">Anyway, now that we're mutating the PMU state before suspending we need<br class="">to be more careful. So I think we need a lock around the whole<br class="">sequence.<br class=""></blockquote></blockquote><br class="">Regardless of the outcome here, generally agreed that some serialization<br class="">should be imposed in this path. The way the platform works (and some<br class="">extra measures by the drmgr utility) make it so that this code isn't<br class="">entered concurrently in usual operation, but it's possible to make it<br class="">happen if you are root.<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="">Yeah I agree it's unlikely to be a problem in practice.</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=""><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="">A file-static mutex should be OK.<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="">Ack.</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=""><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=""><blockquote type="cite" class="">My concern is still that we wouldn't necessarily have the other CPUs<span class="Apple-converted-space"> </span><br class="">under control at that point even if we serialize the migrate path.<br class="">They could take interrupts, possibly call into perf subsystem after<br class="">the mobility_pmu_disable (e.g., via syscall or context switch) which<span class="Apple-converted-space"> </span><br class="">might mess things up.<br class=""><br class="">I think the stop machine is a reasonable place for the code in this<span class="Apple-converted-space"> </span><br class="">case. It's a low level disabling of hardware facility and saving off<span class="Apple-converted-space"> </span><br class="">registers.<br class=""></blockquote><br class="">That makes sense, but I can't help feeling concerned still. For this to<br class="">be safe, power_pmu_enable() and power_pmu_disable() must never sleep or<br class="">re-enable interrupts or send IPIs. I don't see anything obviously unsafe<br class="">right now, but is that already part of their contract? Is there much<br class="">risk they could change in the future to violate those constraints?<br class=""><br class="">That aside, the proposed change seems like we would be hacking around a<br class="">more generic perf/pmu limitation in a powerpc-specific way. I see the<br class="">same behavior on x86 across suspend/resume.<br class=""><br class=""># perf stat -a -e cache-misses -I 1000 & sleep 2 ; systemctl suspend ; sleep 20 ; kill $(jobs -p)<br class="">[1] 189806<br class=""># time counts unit events<br class=""> 1.000501710 9,983,649 cache-misses<br class=""> 2.002620321 14,131,072 cache-misses<br class=""> 3.004579071 23,010,971 cache-misses<br class=""> 9.971854783 140,737,491,680,853 cache-misses<br class=""> 10.982669250 0 cache-misses<br class=""> 11.984660498 0 cache-misses<br class=""> 12.986648392 0 cache-misses<br class=""> 13.988561766 0 cache-misses<br class=""> 14.992670615 0 cache-misses<br class=""> 15.994938111 0 cache-misses<br class=""> 16.996703952 0 cache-misses<br class=""> 17.999092812 0 cache-misses<br class=""> 19.000602677 0 cache-misses<br class=""> 20.003272216 0 cache-misses<br class=""> 21.004770295 0 cache-misses<br class=""># uname -r<br class="">5.13.19-100.fc33.x86_64<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="">That is interesting.</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="">Athira, I guess we should bring that to the perf maintainers and see if</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="">there's any interest in solving the issue in a generic fashion.</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>Sure, will work on proposal to start discussion with the community.</div><div><br class=""></div><div>Thanks</div><div>Athira <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=""><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="">cheers</span></div></blockquote></div><br class=""></div></body></html>