| Message ID | patch-ipi-2-v2@bga.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | Paul Mackerras |
| Headers | show |
Milton Miller writes: > With the new generic smp call function helpers, I noticed the code in > smp_message_recv was a single function call in many cases. While > getting the message number from the ipi data is easy, we can reduce > the path length by a function and data dependent switch by registering > separate ipi actions for these simple calls. With this I get: CC arch/powerpc/kernel/smp.o arch/powerpc/kernel/smp.c: In function 'smp_request_message_ipi': arch/powerpc/kernel/smp.c:177: error: 'ipi_names' undeclared (first use in this function) arch/powerpc/kernel/smp.c:177: error: (Each undeclared identifier is reported only once arch/powerpc/kernel/smp.c:177: error: for each function it appears in.) make[2]: *** [arch/powerpc/kernel/smp.o] Error 1 I think you need smp_ipi_name rather than ipi_names here: > + WARN(err < 0, "unable to request_irq %d for %s (rc %d)\n", > + virq, ipi_names[msg], err); Paul.
On Nov 5, 2008, at 10:42 PM, Paul Mackerras wrote: > Milton Miller writes: > >> With the new generic smp call function helpers, I noticed the code in >> smp_message_recv was a single function call in many cases. While >> getting the message number from the ipi data is easy, we can reduce >> the path length by a function and data dependent switch by registering >> separate ipi actions for these simple calls. > > With this I get: > > CC arch/powerpc/kernel/smp.o > arch/powerpc/kernel/smp.c: In function 'smp_request_message_ipi': > arch/powerpc/kernel/smp.c:177: error: 'ipi_names' undeclared (first > use in this function) > arch/powerpc/kernel/smp.c:177: error: (Each undeclared identifier is > reported only once > arch/powerpc/kernel/smp.c:177: error: for each function it appears in.) > make[2]: *** [arch/powerpc/kernel/smp.o] Error 1 > > I think you need smp_ipi_name rather than ipi_names here: > >> + WARN(err < 0, "unable to request_irq %d for %s (rc %d)\n", >> + virq, ipi_names[msg], err); > You are exactly correct. I changed my mind on the name of this varable, and missed this spot on the final edit. However, my compile passed because it was during the time that the powerpc version of WARN was never evaluating its printf arguments. I wrote and compile tested the change, but need to find or setup a mailer to send it. milton
Index: next.git/arch/powerpc/include/asm/smp.h =================================================================== --- next.git.orig/arch/powerpc/include/asm/smp.h 2008-10-05 00:08:36.000000000 -0500 +++ next.git/arch/powerpc/include/asm/smp.h 2008-10-05 00:16:34.000000000 -0500 @@ -81,6 +81,13 @@ extern int cpu_to_core_id(int cpu); #define PPC_MSG_CALL_FUNC_SINGLE 2 #define PPC_MSG_DEBUGGER_BREAK 3 +/* + * irq controllers that have dedicated ipis per message and don't + * need additional code in the action handler may use this + */ +extern int smp_request_message_ipi(int virq, int message); +extern const char *smp_ipi_name[]; + void smp_init_iSeries(void); void smp_init_pSeries(void); void smp_init_cell(void); Index: next.git/arch/powerpc/kernel/smp.c =================================================================== --- next.git.orig/arch/powerpc/kernel/smp.c 2008-10-05 00:08:38.000000000 -0500 +++ next.git/arch/powerpc/kernel/smp.c 2008-10-05 00:19:12.000000000 -0500 @@ -123,6 +123,65 @@ void smp_message_recv(int msg) } } +static irqreturn_t call_function_action(int irq, void *data) +{ + generic_smp_call_function_interrupt(); + return IRQ_HANDLED; +} + +static irqreturn_t reschedule_action(int irq, void *data) +{ + /* we just need the return path side effect of checking need_resched */ + return IRQ_HANDLED; +} + +static irqreturn_t call_function_single_action(int irq, void *data) +{ + generic_smp_call_function_single_interrupt(); + return IRQ_HANDLED; +} + +static irqreturn_t debug_ipi_action(int irq, void *data) +{ + smp_message_recv(PPC_MSG_DEBUGGER_BREAK); + return IRQ_HANDLED; +} + +static irq_handler_t smp_ipi_action[] = { + [PPC_MSG_CALL_FUNCTION] = call_function_action, + [PPC_MSG_RESCHEDULE] = reschedule_action, + [PPC_MSG_CALL_FUNC_SINGLE] = call_function_single_action, + [PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action, +}; + +const char *smp_ipi_name[] = { + [PPC_MSG_CALL_FUNCTION] = "ipi call function", + [PPC_MSG_RESCHEDULE] = "ipi reschedule", + [PPC_MSG_CALL_FUNC_SINGLE] = "ipi call function single", + [PPC_MSG_DEBUGGER_BREAK] = "ipi debugger", +}; + +/* optional function to request ipi, for controllers with >= 4 ipis */ +int smp_request_message_ipi(int virq, int msg) +{ + int err; + + if (msg < 0 || msg > PPC_MSG_DEBUGGER_BREAK) { + return -EINVAL; + } +#if !defined(CONFIG_DEBUGGER) && !defined(CONFIG_KEXEC) + if (msg == PPC_MSG_DEBUGGER_BREAK) { + return 1; + } +#endif + err = request_irq(virq, smp_ipi_action[msg], IRQF_DISABLED|IRQF_PERCPU, + smp_ipi_name[msg], 0); + WARN(err < 0, "unable to request_irq %d for %s (rc %d)\n", + virq, ipi_names[msg], err); + + return err; +} + void smp_send_reschedule(int cpu) { if (likely(smp_ops))
With the new generic smp call function helpers, I noticed the code in smp_message_recv was a single function call in many cases. While getting the message number from the ipi data is easy, we can reduce the path length by a function and data dependent switch by registering separate ipi actions for these simple calls. Originally I left the ipi action array exposed, but then I realized the registration code should be common too. The three users each had their own name array, so I made a fourth to convert all users to use a common one. Signed-off-by: Milton Miller <miltonm@bga.com> --- v2: fix arg reversal noted by Geert Uytterhoeven. This order is most similar to request_irq and message in data field, but without casting. Perhaps we should make the common code look like an ipi action handler and remove this last call? Currently we still have to allocate a stack frame to load the return value.