perf_events: fix bug in AMD per-cpu initialization
in [Kernel]
|
Prev: [RFC/PATCH] platform_bus: allow custom extensions to system PM methods
Next: platform_bus: allow custom extensions to system PM methods
From: Peter Zijlstra on 17 Mar 2010 19:50 On Wed, 2010-03-17 at 10:40 +0200, Stephane Eranian wrote: > On AMD processors, we need to allocate a data structure per Northbridge > to handle certain events. > > On CPU initialization, we need to query the Northbridge id and check > whether the structure is already allocated or not. We use the > amd_get_nb_id() function to request the Northbridge identification. > > The recent cleanup of the CPU online/offline initialization introduced > a bug. AMD cpu initialization is invoked on CPU_UP_PREPARE callback. > This is before the CPU Northbridge id is calculated. Therefore no > processor had a Northbridge structure allocated except the boot > processor. That was causing bogus NB event scheduling. > > This patch uses the CPU_ONLINE callback to initialize the AMD > Northbridge structure. This way amd_get_nb_id() returns valid > information. > > The x86_cpu_up() callback was added. Could not call it cpu_online > because of existing macro. > > Signed-off-by: Stephane Eranian <eranian(a)google.com> No, ONLINE is not exposed for a good reason, its always wrong. Use prepare to allocate data, and starting to initialize stuff on the cpu proper once its up. Online is after the cpu is up and running and we are already scheduling stuff on it so its too late to initialize things. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Stephane Eranian on 17 Mar 2010 20:40 On Thu, Mar 18, 2010 at 12:47 AM, Peter Zijlstra <peterz(a)infradead.org> wrote: > On Wed, 2010-03-17 at 10:40 +0200, Stephane Eranian wrote: >> On AMD processors, we need to allocate a data structure per Northbridge >> to handle certain events. >> >> On CPU initialization, we need to query the Northbridge id and check >> whether the structure is already allocated or not. We use the >> amd_get_nb_id() function to request the Northbridge identification. >> >> The recent cleanup of the CPU online/offline initialization introduced >> a bug. AMD cpu initialization is invoked on CPU_UP_PREPARE callback. >> This is before the CPU Northbridge id is calculated. Therefore no >> processor had a Northbridge structure allocated except the boot >> processor. That was causing bogus NB event scheduling. >> >> This patch uses the CPU_ONLINE callback to initialize the AMD >> Northbridge structure. This way amd_get_nb_id() returns valid >> information. >> >> The x86_cpu_up() callback was added. Could not call it cpu_online >> because of existing macro. >> >> Signed-off-by: Stephane Eranian <eranian(a)google.com> > > > No, ONLINE is not exposed for a good reason, its always wrong. > > Use prepare to allocate data, and starting to initialize stuff on the > cpu proper once its up. Online is after the cpu is up and running and we > are already scheduling stuff on it so its too late to initialize things. > > I can't because amd_get_nb_id() returns garbage for the CPU when you call from CPU_STARTING. So looks like initialization of cpu_llc_id needs to be moved way earlier. I don't want to have to duplicate that code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 23 Mar 2010 11:10 On Tue, 2010-03-23 at 15:55 +0100, Stephane Eranian wrote: > What's the point of CPU_ONLINE vs. CPU_STARTING if you're saying the > former is never right? Why not move CPU_ONLINE to the right place and > drop CPU_STARTING? Its right for a lot of things, just not for perf, we need to be ready and done by the time the cpu starts scheduling. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 23 Mar 2010 11:20 On Tue, 2010-03-23 at 16:12 +0100, Stephane Eranian wrote: > On Tue, Mar 23, 2010 at 4:07 PM, Peter Zijlstra <peterz(a)infradead.org> wrote: > > On Tue, 2010-03-23 at 15:55 +0100, Stephane Eranian wrote: > >> What's the point of CPU_ONLINE vs. CPU_STARTING if you're saying the > >> former is never right? Why not move CPU_ONLINE to the right place and > >> drop CPU_STARTING? > > > > Its right for a lot of things, just not for perf, we need to be ready > > and done by the time the cpu starts scheduling. > > > You mean they need to wait until after the cpu starts scheduling? > As opposed to being called just before it starts scheduling. As in it doesn't really matter for them, and the CPU_ONLINE call is convenient in that it allows the callback to schedule too. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Stephane Eranian on 23 Mar 2010 11:20
On Tue, Mar 23, 2010 at 4:07 PM, Peter Zijlstra <peterz(a)infradead.org> wrote: > On Tue, 2010-03-23 at 15:55 +0100, Stephane Eranian wrote: >> What's the point of CPU_ONLINE vs. CPU_STARTING if you're saying the >> former is never right? Why not move CPU_ONLINE to the right place and >> drop CPU_STARTING? > > Its right for a lot of things, just not for perf, we need to be ready > and done by the time the cpu starts scheduling. > You mean they need to wait until after the cpu starts scheduling? As opposed to being called just before it starts scheduling. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |