| Message ID | 20181227111821.80908-3-anup@brainfault.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IRQ affinity support in PLIC driver | expand |
On Thu, Dec 27, 2018 at 04:48:18PM +0530, Anup Patel wrote: > The plic_toggle() uses raw_spin_lock() and plic_irq_toggle has a > for loop so both these functions are not suitable for being inline > hence this patch removes the inline keyword. That is a weird argument for a function which has by design exactly two callers and is in the hot path. The alternative to the inline here would be to duplicate the code.
On Tue, Jan 15, 2019 at 9:24 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Thu, Dec 27, 2018 at 04:48:18PM +0530, Anup Patel wrote: > > The plic_toggle() uses raw_spin_lock() and plic_irq_toggle has a > > for loop so both these functions are not suitable for being inline > > hence this patch removes the inline keyword. > > That is a weird argument for a function which has by design exactly > two callers and is in the hot path. The alternative to the inline > here would be to duplicate the code. It's strange that you see it as weird argument. Both plic_toggle() and plic_irq_toggle() are 5+ lines functions with loops. The loop is clear in plic_irq_toggle() whereas raw_spin_lock() in plic_toggle() expands into inline-assembly spin-loop because raw_spin_lock() is a macro (not function). Further looking at disassembly of both functions, these are 55+ instructions. I think we let GCC decide whether these functions should be inlined or not rather than us explicitly making these functions inline. Regards, Anup
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index c23a293a2aae..01bbbbffbcae 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -69,8 +69,8 @@ struct plic_handler { }; static DEFINE_PER_CPU(struct plic_handler, plic_handlers); -static inline void plic_toggle(struct plic_handler *handler, - int hwirq, int enable) +static void plic_toggle(struct plic_handler *handler, + int hwirq, int enable) { u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32); u32 hwirq_mask = 1 << (hwirq % 32); @@ -83,7 +83,7 @@ static inline void plic_toggle(struct plic_handler *handler, raw_spin_unlock(&handler->enable_lock); } -static inline void plic_irq_toggle(struct irq_data *d, int enable) +static void plic_irq_toggle(struct irq_data *d, int enable) { int cpu;
The plic_toggle() uses raw_spin_lock() and plic_irq_toggle has a for loop so both these functions are not suitable for being inline hence this patch removes the inline keyword. Signed-off-by: Anup Patel <anup@brainfault.org> --- drivers/irqchip/irq-sifive-plic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)