Hi,
This is a repost of a couple patches I posted about a month ago related to
porting perfmon2 to powerpc. I wanted to see if there were any further
comments on these patches, and also wanted to ask if these should be
submitted separately to the ppc kernel maintainers, or if they should be kept
with the perfmon2 patches until those are submitted.
The first patch adds an smp_call_function_single() routine for powerpc. In
2.6.21, there's a prototype for this routine in include/linux/smp.h, and it
is implemented on i386, ia64, and x86-64. Since this routine is very similar
to the existing smp_call_function() routine, the common portions have been
pulled out into __smp_call_function(). Perfmon2 uses this unload a context
that's loaded on a CPU other than the one that's performing the close-context
routine.
The second patch changes the powerpc version of topology_init() from an
__initcall to a subsys_initcall, which matches the definition of
topology_init() on all other architectures. Perfmon2's initialization is done
as a subsys_initcall, but it fails to set up its sysfs information if
topology_init() has not yet run. Changing perfmon2 to an __initcall() was
discussed, but we came to the conclusion that this would not work because the
perfmon2 core must be initialized before any of its sub-modules. These
sub-modules are located in arch/*/perfmon/, and thus if they are built
statically they would run their module_init routines before the perfmon2 core
ran its __initcall routine.
We have been running tests with these patches for the last few weeks on Cell
and Power5 systems without any problems.
Thanks,
--
Kevin Corry
[email protected]
http://www.ibm.com/linux/
Add an smp_call_function_single() to the powerpc architecture. Since this
is very similar to the existing smp_call_function() routine, the common
portions have been split out into __smp_call_function(). Since the
spin_lock(&call_lock) was moved to __smp_call_function(),
smp_call_function() now explicitly calls preempt_disable() before getting
the count of online CPUs.
Signed-off-by: Kevin Corry <[email protected]>
Index: linux-2.6.21/arch/powerpc/kernel/smp.c
===================================================================
--- linux-2.6.21.orig/arch/powerpc/kernel/smp.c
+++ linux-2.6.21/arch/powerpc/kernel/smp.c
@@ -175,26 +175,11 @@ static struct call_data_struct {
/* delay of at least 8 seconds */
#define SMP_CALL_TIMEOUT 8
-/*
- * This function sends a 'generic call function' IPI to all other CPUs
- * in the system.
- *
- * [SUMMARY] Run a function on all other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
- *
- * You must not call this function with disabled interrupts or from a
- * hardware interrupt handler or from a bottom half handler.
- */
-int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
- int wait)
-{
+static int __smp_call_function(void (*func)(void *info), void *info,
+ int wait, int target_cpu, int num_cpus)
+{
struct call_data_struct data;
- int ret = -1, cpus;
+ int ret = -1;
u64 timeout;
/* Can deadlock when called with interrupts disabled */
@@ -211,40 +196,33 @@ int smp_call_function (void (*func) (voi
atomic_set(&data.finished, 0);
spin_lock(&call_lock);
- /* Must grab online cpu count with preempt disabled, otherwise
- * it can change. */
- cpus = num_online_cpus() - 1;
- if (!cpus) {
- ret = 0;
- goto out;
- }
call_data = &data;
smp_wmb();
/* Send a message to all other CPUs and wait for them to respond */
- smp_ops->message_pass(MSG_ALL_BUT_SELF, PPC_MSG_CALL_FUNCTION);
+ smp_ops->message_pass(target_cpu, PPC_MSG_CALL_FUNCTION);
timeout = get_tb() + (u64) SMP_CALL_TIMEOUT * tb_ticks_per_sec;
/* Wait for response */
- while (atomic_read(&data.started) != cpus) {
+ while (atomic_read(&data.started) != num_cpus) {
HMT_low();
if (get_tb() >= timeout) {
- printk("smp_call_function on cpu %d: other cpus not "
- "responding (%d)\n", smp_processor_id(),
- atomic_read(&data.started));
+ printk("%s on cpu %d: other cpus not "
+ "responding (%d)\n", __FUNCTION__,
+ smp_processor_id(), atomic_read(&data.started));
debugger(NULL);
goto out;
}
}
if (wait) {
- while (atomic_read(&data.finished) != cpus) {
+ while (atomic_read(&data.finished) != num_cpus) {
HMT_low();
if (get_tb() >= timeout) {
- printk("smp_call_function on cpu %d: other "
- "cpus not finishing (%d/%d)\n",
- smp_processor_id(),
+ printk("%s on cpu %d: other cpus "
+ "not finishing (%d/%d)\n",
+ __FUNCTION__, smp_processor_id(),
atomic_read(&data.finished),
atomic_read(&data.started));
debugger(NULL);
@@ -262,8 +240,74 @@ int smp_call_function (void (*func) (voi
return ret;
}
+/*
+ * This function sends a 'generic call function' IPI to all other CPUs
+ * in the system.
+ *
+ * [SUMMARY] Run a function on all other CPUs.
+ * <func> The function to run. This must be fast and non-blocking.
+ * <info> An arbitrary pointer to pass to the function.
+ * <nonatomic> currently unused.
+ * <wait> If true, wait (atomically) until function has completed on other CPUs.
+ * [RETURNS] 0 on success, else a negative status code. Does not return until
+ * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ *
+ * You must not call this function with disabled interrupts or from a
+ * hardware interrupt handler or from a bottom half handler.
+ */
+int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
+ int wait)
+{
+ int num_cpus, ret = 0;
+
+ /* Must grab online cpu count with preempt disabled, otherwise
+ * it can change. */
+ preempt_disable();
+ num_cpus = num_online_cpus() - 1;
+ if (num_cpus) {
+ ret = __smp_call_function(func, info, wait,
+ MSG_ALL_BUT_SELF, num_cpus);
+ }
+ preempt_enable();
+ return ret;
+}
+
EXPORT_SYMBOL(smp_call_function);
+/*
+ * This function sends a 'generic call function' IPI to the specified CPU.
+ *
+ * [SUMMARY] Run a function on the specified CPUs.
+ * <cpuid> The CPU to run the function on.
+ * <func> The function to run. This must be fast and non-blocking.
+ * <info> An arbitrary pointer to pass to the function.
+ * <nonatomic> currently unused.
+ * <wait> If true, wait (atomically) until function has completed on the
+ * other CPU.
+ * [RETURNS] 0 on success, else a negative status code. Does not return until
+ * remote CPU is nearly ready to execute <<func>> or are or has executed.
+ *
+ * You must not call this function with disabled interrupts or from a
+ * hardware interrupt handler or from a bottom half handler.
+ */
+int smp_call_function_single(int cpuid, void (*func)(void *info), void *info,
+ int nonatomic, int wait)
+{
+ int ret;
+
+ /* Prevent preemption and reschedule on another processor */
+ if (get_cpu() == cpuid) {
+ printk(KERN_INFO "%s: trying to call self\n", __FUNCTION__);
+ ret = -EBUSY;
+ } else
+ ret = __smp_call_function(func, info, wait, cpuid, 1);
+
+ put_cpu();
+ return ret;
+}
+
+EXPORT_SYMBOL(smp_call_function_single);
+
void smp_call_function_interrupt(void)
{
void (*func) (void *info);
Change the powerpc version of topology_init() from an __initcall to
a subsys_initcall to match all other architectures.
Signed-off-by: Kevin Corry <[email protected]>
Index: linux-2.6.21/arch/powerpc/kernel/sysfs.c
===================================================================
--- linux-2.6.21.orig/arch/powerpc/kernel/sysfs.c
+++ linux-2.6.21/arch/powerpc/kernel/sysfs.c
@@ -498,4 +498,4 @@ static int __init topology_init(void)
return 0;
}
-__initcall(topology_init);
+subsys_initcall(topology_init);
On Wed, 2007-05-02 at 12:11 -0500, Kevin Corry wrote:
> Change the powerpc version of topology_init() from an __initcall to
> a subsys_initcall to match all other architectures.
>
> Signed-off-by: Kevin Corry <[email protected]>
>
> Index: linux-2.6.21/arch/powerpc/kernel/sysfs.c
> ===================================================================
> --- linux-2.6.21.orig/arch/powerpc/kernel/sysfs.c
> +++ linux-2.6.21/arch/powerpc/kernel/sysfs.c
> @@ -498,4 +498,4 @@ static int __init topology_init(void)
>
> return 0;
> }
> -__initcall(topology_init);
> +subsys_initcall(topology_init);
topology_init() depends on the register_one_node() stuff being
available, which relies on register_node_type() being called AFAICT -
which is a postcore_initcall(). So that's OK.
It also creates sysfs files, which is OK because long before initcalls
run vfs_caches_init() called mnt_init() which called sysfs_init().
Just to be super safe it'd be good to diff your sysfs before and after
the change. But assuming that show's nothing this looks fine to me.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Hi Michael,
On Wed May 2 2007 8:11 pm, Michael Ellerman wrote:
> On Wed, 2007-05-02 at 12:11 -0500, Kevin Corry wrote:
> > Change the powerpc version of topology_init() from an __initcall to
> > a subsys_initcall to match all other architectures.
> >
> > Signed-off-by: Kevin Corry <[email protected]>
> >
> > Index: linux-2.6.21/arch/powerpc/kernel/sysfs.c
> > ===================================================================
> > --- linux-2.6.21.orig/arch/powerpc/kernel/sysfs.c
> > +++ linux-2.6.21/arch/powerpc/kernel/sysfs.c
> > @@ -498,4 +498,4 @@ static int __init topology_init(void)
> >
> > return 0;
> > }
> > -__initcall(topology_init);
> > +subsys_initcall(topology_init);
>
> topology_init() depends on the register_one_node() stuff being
> available, which relies on register_node_type() being called AFAICT -
> which is a postcore_initcall(). So that's OK.
>
> It also creates sysfs files, which is OK because long before initcalls
> run vfs_caches_init() called mnt_init() which called sysfs_init().
>
> Just to be super safe it'd be good to diff your sysfs before and after
> the change. But assuming that show's nothing this looks fine to me.
I booted with and without this patch applied, and the file listings of /sys on
both kernels show no differences (other than timestamps).
Thanks for the feedback.
--
Kevin Corry
[email protected]
http://www.ibm.com/linux/