Messages in this thread | | | Date | Fri, 24 Nov 2023 00:50:52 +0000 | From | Mingwei Zhang <> | Subject | Re: [PATCH 1/3] perf/core: Update perf_adjust_freq_unthr_context() |
| |
On Tue, Nov 21, 2023, Mingwei Zhang wrote: > On Tue, Nov 21, 2023, Namhyung Kim wrote: > > > Hi Namhyung, > > > Hi Mingwei, > > > > On Mon, Nov 20, 2023 at 3:24 PM Mingwei Zhang <mizhang@google.com> wrote: > > > > > > On Mon, Nov 20, 2023, Ian Rogers wrote: > > > > On Mon, Nov 20, 2023 at 2:19 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > > > It was unnecessarily disabling and enabling PMUs for each event. It > > > > > should be done at PMU level. Add pmu_ctx->nr_freq counter to check it > > > > > at each PMU. As pmu context has separate active lists for pinned group > > > > > and flexible group, factor out a new function to do the job. > > > > > > > > > > Another minor optimization is that it can skip PMUs w/ CAP_NO_INTERRUPT > > > > > even if it needs to unthrottle sampling events. > > > > > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > Series: > > > > Reviewed-by: Ian Rogers <irogers@google.com> > > > > > > > > Thanks, > > > > Ian > > > > > > > > > > Can we have "Cc: stable@vger.kernel.org" for the whole series? This > > > series should have a great performance improvement for all VMs in which > > > perf sampling events without specifying period. > > > > I was not sure if it's ok to have this performance fix in the stable series. > > > > Critical performance bug fix is ok to be added to stable tree, as the > requirements are mentioned here: > > https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst > > In particular, this patch satisfies the 2nd sub-bullet of the forth > bullet. > > But let me step back. Only this patch is needed with stable tag instead > of the whole series. This patch impact 69 lines of code. It satisfies > the rule of within 100 lines (bullet 3). > > I will give a try and test it today or tomorrow and make sure we satisfy > bullet 2. > > Once it gets in, bullet 1 will be satisfied as well. > > Overall, the intention is to improve PMU performance in VM as early as > we can since we don't control the schedule of distro kernel upgrade and > we don't control when end customers upgrade their running kernel. So I > presume even adding to the stable tree may take years to see the result > change. But if we don't do it, it may take way longer (since it does not > contain a "Fixes" tag as well). > > Thanks. > -Mingwei >
I have tested the code. Yes profiling results in the VM shows that it removes perf_adjust_freq_unthr_contex() as the hot spot. However, when running perf with sufficient events in frequency mode that triggers multiplexing. The overall performance overhead still reaches 60% per CPU (this overhead is invisible to vCPU).
At the host level, I have been monitoring the write MSRs and found that the repeated writes to 0x38f disappeared, indicating that this patch is indeed working. But on the other hand, I have noticed more frequent overflows and PMIs.
The more frequent overflows is shown in the writes to MSR 0x390 and reads to MSR 0x38e. I infer the more frequent PMIs from the much longer execution of 'vmx_vmexit()' shown in flamegraph.
Because of the above observation, it seems to me that this patch no longer satisfies the requirements of Ccing "stable@vger.kernel.org". I will double check and follow up on this one.
Thanks. -Mingwei
| |