Kevin, On Thu, Apr 05, 2007 at 01:29:35PM -0500, Kevin Corry wrote: > > Just wanted to send in a quick update on the progress of the powerpc/cell > support. Carl and I have been plugging away at getting the basic support > done, and after getting past a couple compile problems, we were able to get a > kernel that would start to boot. But it crashed pretty early on while trying > to register the default sampling-format module. > > The pfm_init() routine is the global perfmon initialization during bootup. > This routine calls pfm_init_sysfs(), which initializes all the basic perfmon > files in sysfs. This consists of registering some subsystems, adding a couple > kobjects, and creating a sysfs group. Later in that routine it calls > pfm_sysfs_add_cpu() for each online CPU. The first thing that routine does is > call get_cpu_sysdev() to get the sys_device structure for the desired CPU. > However, when this runs (on a POWER5 box and a Cell box), it gets a NULL back > from that routine, which returns an error to pfm_init_sysfs(), which causes > all the sysfs setup to be torn down. An error is returned, which is ignored > by pfm_init(), since the comment says that sysfs is not critical for normal > operations. However, this causes a segfault later when the default > sampling-format tries to register, which in turn tries to add a new kobject > to the kobjects that were supposed to be created in pfm_init_sysfs(). > I will certainly fix that, it should not crash.
> So why are we getting NULL pointers back from get_cpu_sysdev()? That routine > gets the desired pointer from an array that is set up in arch-specific code, > usually in the routine register_cpu(), which is usually called from the > routine topology_init(). On all architectures except powerpc, the > topology_init() routine is declared as a subsys_initcall() routine. On > powerpc, however, it's declared as just an __initcall() routine (which > translates to device_initcall(), which is also equivalent to module_init() - > see include/linux/init.h for all the gory details). The problem here is that > pfm_init() is declared as a subsys_initcall() routine, which means it's > always going to run before topology_init() on powerpc systems, and thus will > always get an error when trying to set up the sysfs info. > > So the obvious fix is to change topology_init() (in > arch/powerpc/kernel/sysfs.c) to a subsys_initcall(). This works on our > systems (and gets us to another segfault which I'll describe in a later > email). However, I haven't yet consulted with any of the powerpc kernel > developers to see if this change would break anything else in the powerpc > arch code (I'll let you know what I find out from them). I would think that > making this change would be ok (especially since it's done that way on every > other arch), but in the event that there's a specific reason for keeping it > as-is, we may need to change pfm_init() to a device_initcall() and figure out > another way to ensure that pfm_init() runs before any Perfmon-related modules > try to register with the Perfmon core. > Ok, let us know what you find out about powerpc. I suspect we should be fine with pfm_inti() happening at device_initcall() rather than subsys_initcall(), though I'd to run the checks (and on all archictures we support!). > On a related note, it would seem that sysfs *is* critical for normal > operation, since we're seeing segfaults when Perfmon's sysfs info is not > available. Perhaps we should check the return value from pfm_init_sysfs() in > pfm_init(). Alternatively, we'd have to do a better job of checking whether > the sysfs info was setup correctly any time we try to access a kobject in any > of the Perfmon data structures. I've attached a patch below for the first > soluation. Will fix that ASAP. Thanks for chasing down these problems and keeping us informed on your progress. -- -Stephane _______________________________________________ perfmon mailing list [email protected] http://www.hpl.hp.com/hosted/linux/mail-archives/perfmon/
