Re: [PATCH v2] arm64: perf: Fix access percpu variables in preemptible context

From: Will Deacon <will@kernel.org>
To: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: mark.rutland@arm.com, sumit.garg@linaro.org,
	alexander.shishkin@linux.intel.com, alexandru.elisei@arm.com,
	yj.chiang@mediatek.com, linux-kernel@vger.kernel.org,
	acme@kernel.org, oliver.sang@intel.com, peterz@infradead.org,
	mingo@redhat.com, linux-mediatek@lists.infradead.org,
	catalin.marinas@arm.com, matthias.bgg@gmail.com,
	namhyung@kernel.org, jolsa@redhat.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2] arm64: perf: Fix access percpu variables in preemptible context
Date: Tue, 12 Jan 2021 15:07:36 +0000	[thread overview]
Message-ID: <20210112150735.GC9689@willie-the-truck> (raw)
In-Reply-To: <20210108125527.14175-1-lecopzer.chen@mediatek.com>

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?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel