Messages in this thread | | | Date | Tue, 12 Jan 2021 16:32:54 +0000 | From | Mark Rutland <> | Subject | Re: [PATCH v2] arm64: perf: Fix access percpu variables in preemptible context |
| |
On Tue, Jan 12, 2021 at 03:07:36PM +0000, Will Deacon wrote: > On Fri, Jan 08, 2021 at 08:55:27PM +0800, Lecopzer Chen wrote: > > > On Mon, 21 Dec 2020 at 21:53, Lecopzer Chen <lecopzer.chen@mediatek.com> wrote: > > > > > > > > commit 367c820ef08082 ("arm64: Enable perf events based hard lockup detector") > > > > reinitilizes lockup detector after arm64 PMU is initialized and open > > > > a window for accessing smp_processor_id() in preemptible context. > > > > Since hardlockup_detector_perf_init() always called in init stage > > > > with a single cpu, but we initialize lockup detector after the init task > > > > is migratable. > > > > > > > > Fix this by utilizing lockup detector reconfiguration which calls > > > > softlockup_start_all() on each cpu and calls watatchdog_nmi_enable() later. > > > > Because softlockup_start_all() use IPI call function to make sure > > > > watatchdog_nmi_enable() will bind on each cpu and fix this issue. > > > > > > IMO, this just creates unnecessary dependency for hardlockup detector > > > init via softlockup detector (see the alternative definition of > > > lockup_detector_reconfigure()). > > > > > > The arm64/Kconfig select HAVE_HARDLOCKUP_DETECTOR_PERF if we have NMI: > > select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI > > > > And in lib/Kconfig.debug HARDLOCKUP_DETECTOR select SOFTLOCKUP_DETECTOR automatically. > > config HARDLOCKUP_DETECTOR_PERF > > bool > > select SOFTLOCKUP_DETECTOR > > > > So we don't need to explicitly select softlockup. > > And actually this patch is not a perfect solution like you said > > (hardlockup depends on softlockup), > > but the key point is that lockup_detector_init() seems only design for > > using in early init stage and not for calling in later deffered initial process. > > I agree; the current usage in armv8_pmu_driver_init() looks very broken to > me, and bodging it with raw_smp_processor_id() isn't the right solution. > > Maybe we should just revert 367c820ef08082, as this looks like a design > issue rather than something with a simple fix?
I think that would make sense for now, then we can reconsider the whole thing rather than looking for a point-fix.
Thanks, Mark.
| |