Hi,

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().

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.

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.

Thanks,
-- 
Kevin Corry
[EMAIL PROTECTED]
http://www.ibm.com/linux/


--- linux-2.6.20-perfmon.orig/perfmon/perfmon.c
+++ linux-2.6.20-perfmon/perfmon/perfmon.c
@@ -868,7 +868,8 @@ int __init pfm_init(void)
         * sysfs is not critical for normal operation, so
         * this is not a fatal error;
         */
-       pfm_init_sysfs();
+       if (pfm_init_sysfs())
+               goto error_disable;
 
        /*
         * one time, arch-specific global initialization
_______________________________________________
perfmon mailing list
[email protected]
http://www.hpl.hp.com/hosted/linux/mail-archives/perfmon/

Reply via email to