[PATCH 0/3] Remove setup_percpu_irq() & remove_percpu_irq

[PATCH 0/3] Remove setup_percpu_irq() & remove_percpu_irq

[afzal mohammed]

Hi,

While doing the removal of setup_irq(), it was observed that
setup_percpu_irq() also can be removed similarly by replacing it w/
request_percpu_irq(), which does allocate memory. In the initial
setup_irq() removal cover letters [1], it was mentioned that
setup_percpu_irq() is untouched.

After removing setup_irq(), it does not look good to let live
setup_percpu_irq(), especially since it being a low hanging fruit. Hence
replace setup_percpu_irq() by it's allocator equivalent.
request_percpu_irq() cannot be used since all the users need to pass
IRQF_TIMER flag, which it would not allow. Thus it's variant,
__request_percpu_irq() is used.

In addition to removing setup_percpu_irq() definition,
remove_percpu_irq(), unused, is also removed.

It seems setup_percpu_irq() is used only by MIPS (even the one outside
MIPS arch dir, i.e. the clocksource driver). The series has been build
tested, though i would have loved to do a qemu test as well, i do not
see a qemu machine type that can test the changes done here (in malta
machine, in which clocksource changes are getting built, the changes
done here are not being runtime tested).

Regards
afzal

[1] https://lkml.kernel.org/r/cover.1581478323.git.afzal.mohd.ma@gmail.com

afzal mohammed (3):
  MIPS: Replace setup_percpu_irq() by request_percpu_irq() variant
  clocksource/drivers/mips-gic-timer: Replace setup_percpu_irq() by
    request_percpu_irq() variant
  genirq: Remove setup_percpu_irq() and remove_percpu_irq()

 arch/mips/include/asm/cevt-r4k.h     |  1 -
 arch/mips/kernel/cevt-r4k.c          | 11 --------
 arch/mips/sgi-ip27/ip27-timer.c      | 13 ++++-----
 arch/mips/sgi-ip30/ip30-timer.c      |  6 ++--
 drivers/clocksource/mips-gic-timer.c | 10 ++-----
 include/linux/irq.h                  |  2 --
 kernel/irq/manage.c                  | 42 ----------------------------
 7 files changed, 10 insertions(+), 75 deletions(-)

-- 
2.18.0

[PATCH 1/3] MIPS: Replace setup_percpu_irq() by request_percpu_irq() variant

[afzal mohammed]

Recently all usages of setup_irq() was replaced by request_irq() as
allocators are ready by the time setup_irq() was invoked. Similarly
setup_percpu_irq() can be replaced by request_percpu_irq(). But in the
callers handled here __request_percpu_irq() has to be used as
request_percpu_irq() does not give the user a chance to pass the flags,
and IRQF_TIMER has to be passed here, so the variant
__request_percpu_irq() is used.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/mips/include/asm/cevt-r4k.h |  1 -
 arch/mips/kernel/cevt-r4k.c      | 11 -----------
 arch/mips/sgi-ip27/ip27-timer.c  | 13 +++++--------
 arch/mips/sgi-ip30/ip30-timer.c  |  6 +++---
 4 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/arch/mips/include/asm/cevt-r4k.h b/arch/mips/include/asm/cevt-r4k.h
index 2e13a038d260..5229eb34f28a 100644
--- a/arch/mips/include/asm/cevt-r4k.h
+++ b/arch/mips/include/asm/cevt-r4k.h
@@ -23,7 +23,6 @@ void mips_event_handler(struct clock_event_device *dev);
 int c0_compare_int_usable(void);
 irqreturn_t c0_compare_interrupt(int, void *);
 
-extern struct irqaction c0_compare_irqaction;
 extern int cp0_timer_irq_installed;
 
 #endif /* __ASM_CEVT_R4K_H */
diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
index 17a9cbb8b3df..4ffa9f485d07 100644
--- a/arch/mips/kernel/cevt-r4k.c
+++ b/arch/mips/kernel/cevt-r4k.c
@@ -158,17 +158,6 @@ irqreturn_t c0_compare_interrupt(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-struct irqaction c0_compare_irqaction = {
-	.handler = c0_compare_interrupt,
-	/*
-	 * IRQF_SHARED: The timer interrupt may be shared with other interrupts
-	 * such as perf counter and FDC interrupts.
-	 */
-	.flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED,
-	.name = "timer",
-};
-
-
 void mips_event_handler(struct clock_event_device *dev)
 {
 }
diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
index 61f3565f3645..6e2b58b47580 100644
--- a/arch/mips/sgi-ip27/ip27-timer.c
+++ b/arch/mips/sgi-ip27/ip27-timer.c
@@ -68,13 +68,6 @@ static irqreturn_t hub_rt_counter_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-struct irqaction hub_rt_irqaction = {
-	.handler	= hub_rt_counter_handler,
-	.percpu_dev_id	= &hub_rt_clockevent,
-	.flags		= IRQF_PERCPU | IRQF_TIMER,
-	.name		= "hub-rt",
-};
-
 /*
  * This is a hack; we really need to figure these values out dynamically
  *
@@ -111,9 +104,13 @@ void hub_rt_clock_event_init(void)
 
 static void __init hub_rt_clock_event_global_init(void)
 {
+	int irq = IP27_RT_TIMER_IRQ;
+
 	irq_set_handler(IP27_RT_TIMER_IRQ, handle_percpu_devid_irq);
 	irq_set_percpu_devid(IP27_RT_TIMER_IRQ);
-	setup_percpu_irq(IP27_RT_TIMER_IRQ, &hub_rt_irqaction);
+	if (__request_percpu_irq(irq, hub_rt_counter_handler, IRQF_TIMER,
+				 "hub-rt", &hub_rt_clockevent) < 0)
+		pr_err("Failed to request percpu irq %d (hub-rt)\n", irq);
 }
 
 static u64 hub_rt_read(struct clocksource *cs)
diff --git a/arch/mips/sgi-ip30/ip30-timer.c b/arch/mips/sgi-ip30/ip30-timer.c
index d13e105478ae..dcc22eaddcda 100644
--- a/arch/mips/sgi-ip30/ip30-timer.c
+++ b/arch/mips/sgi-ip30/ip30-timer.c
@@ -52,11 +52,11 @@ void __init plat_time_init(void)
 	int irq = get_c0_compare_int();
 
 	cp0_timer_irq_installed = 1;
-	c0_compare_irqaction.percpu_dev_id = &mips_clockevent_device;
-	c0_compare_irqaction.flags &= ~IRQF_SHARED;
 	irq_set_handler(irq, handle_percpu_devid_irq);
 	irq_set_percpu_devid(irq);
-	setup_percpu_irq(irq, &c0_compare_irqaction);
+	if (__request_percpu_irq(irq, c0_compare_interrupt, IRQF_TIMER, "timer",
+				 &mips_clockevent_device) < 0)
+		pr_err("Failed to request percpu irq %d (timer)\n", irq);
 	enable_percpu_irq(irq, IRQ_TYPE_NONE);
 
 	ip30_heart_clocksource_init();
-- 
2.18.0

[PATCH 2/3] clocksource/drivers/mips-gic-timer: Replace setup_percpu_irq() by request_percpu_irq() variant

[afzal mohammed]

Recently all usages of setup_irq() was replaced by request_irq() as
allocators are ready by the time setup_irq() was invoked. Similarly
setup_percpu_irq() can be replaced by request_percpu_irq(). But in the
callers handled here __request_percpu_irq() has to be used as
request_percpu_irq() does not give the user a chance to pass the flags,
and IRQF_TIMER has to be passed here, so the variant
__request_percpu_irq() is used.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 drivers/clocksource/mips-gic-timer.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 37671a5d4ed9..e8e6bb6159f5 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -67,13 +67,6 @@ static irqreturn_t gic_compare_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static struct irqaction gic_compare_irqaction = {
-	.handler = gic_compare_interrupt,
-	.percpu_dev_id = &gic_clockevent_device,
-	.flags = IRQF_PERCPU | IRQF_TIMER,
-	.name = "timer",
-};
-
 static void gic_clockevent_cpu_init(unsigned int cpu,
 				    struct clock_event_device *cd)
 {
@@ -137,7 +130,8 @@ static int gic_clockevent_init(void)
 	if (!gic_frequency)
 		return -ENXIO;
 
-	ret = setup_percpu_irq(gic_timer_irq, &gic_compare_irqaction);
+	ret = __request_percpu_irq(gic_timer_irq, gic_compare_interrupt,
+				   IRQF_TIMER, "timer", &gic_clockevent_device);
 	if (ret < 0) {
 		pr_err("IRQ %d setup failed (%d)\n", gic_timer_irq, ret);
 		return ret;
-- 
2.18.0

[PATCH 3/3] genirq: Remove setup_percpu_irq() and remove_percpu_irq()

[afzal mohammed]

Only users of setup_percpu_irq() were in MIPS, they have been converted
to use request_percpu_irq() variants. Also remove_percpu_irq() have no
users.

Hence remove setup_percpu_irq() as well as remove_percpu_irq().

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 include/linux/irq.h |  2 --
 kernel/irq/manage.c | 42 ------------------------------------------
 2 files changed, 44 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 29f5bad87eb3..156995ab5d6e 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -560,8 +560,6 @@ enum {
 #define IRQ_DEFAULT_INIT_FLAGS	ARCH_IRQ_INIT_FLAGS
 
 struct irqaction;
-extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
-extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
 
 extern void irq_cpu_online(void);
 extern void irq_cpu_offline(void);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index aa03b64605d3..22aaa6895c42 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2324,21 +2324,6 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_
 	return NULL;
 }
 
-/**
- *	remove_percpu_irq - free a per-cpu interrupt
- *	@irq: Interrupt line to free
- *	@act: irqaction for the interrupt
- *
- * Used to remove interrupts statically setup by the early boot process.
- */
-void remove_percpu_irq(unsigned int irq, struct irqaction *act)
-{
-	struct irq_desc *desc = irq_to_desc(irq);
-
-	if (desc && irq_settings_is_per_cpu_devid(desc))
-	    __free_percpu_irq(irq, act->percpu_dev_id);
-}
-
 /**
  *	free_percpu_irq - free an interrupt allocated with request_percpu_irq
  *	@irq: Interrupt line to free
@@ -2377,33 +2362,6 @@ void free_percpu_nmi(unsigned int irq, void __percpu *dev_id)
 	kfree(__free_percpu_irq(irq, dev_id));
 }
 
-/**
- *	setup_percpu_irq - setup a per-cpu interrupt
- *	@irq: Interrupt line to setup
- *	@act: irqaction for the interrupt
- *
- * Used to statically setup per-cpu interrupts in the early boot process.
- */
-int setup_percpu_irq(unsigned int irq, struct irqaction *act)
-{
-	struct irq_desc *desc = irq_to_desc(irq);
-	int retval;
-
-	if (!desc || !irq_settings_is_per_cpu_devid(desc))
-		return -EINVAL;
-
-	retval = irq_chip_pm_get(&desc->irq_data);
-	if (retval < 0)
-		return retval;
-
-	retval = __setup_irq(irq, desc, act);
-
-	if (retval)
-		irq_chip_pm_put(&desc->irq_data);
-
-	return retval;
-}
-
 /**
  *	__request_percpu_irq - allocate a percpu interrupt line
  *	@irq: Interrupt line to allocate
-- 
2.18.0

Re: [PATCH 0/3] Remove setup_percpu_irq() & remove_percpu_irq

[afzal mohammed]

Hi Thomas Gleixner,

On Sat, Apr 11, 2020 at 09:34:07PM +0530, afzal mohammed wrote:

> While doing the removal of setup_irq(), it was observed that
> setup_percpu_irq() also can be removed similarly by replacing it w/
> request_percpu_irq(), which does allocate memory. In the initial
> setup_irq() removal cover letters [1], it was mentioned that
> setup_percpu_irq() is untouched.
> 
> After removing setup_irq(), it does not look good to let live
> setup_percpu_irq(), especially since it being a low hanging fruit. Hence
> replace setup_percpu_irq() by it's allocator equivalent.
> request_percpu_irq() cannot be used since all the users need to pass
> IRQF_TIMER flag, which it would not allow. Thus it's variant,
> __request_percpu_irq() is used.
> 
> In addition to removing setup_percpu_irq() definition,
> remove_percpu_irq(), unused, is also removed.

Do you feel that this series adds value ?, if not, i will abandon this
series.

Thanks for your guidance w.r.t setup_irq() removal.

Regards
afzal

Re: [PATCH 0/3] Remove setup_percpu_irq() & remove_percpu_irq

[Thomas Gleixner]

Afzal,

afzal mohammed <afzal.mohd.ma@gmail.com> writes:
> On Sat, Apr 11, 2020 at 09:34:07PM +0530, afzal mohammed wrote:
>
>> While doing the removal of setup_irq(), it was observed that
>> setup_percpu_irq() also can be removed similarly by replacing it w/
>> request_percpu_irq(), which does allocate memory. In the initial
>> setup_irq() removal cover letters [1], it was mentioned that
>> setup_percpu_irq() is untouched.
>> 
>> After removing setup_irq(), it does not look good to let live
>> setup_percpu_irq(), especially since it being a low hanging fruit. Hence
>> replace setup_percpu_irq() by it's allocator equivalent.
>> request_percpu_irq() cannot be used since all the users need to pass
>> IRQF_TIMER flag, which it would not allow. Thus it's variant,
>> __request_percpu_irq() is used.
>> 
>> In addition to removing setup_percpu_irq() definition,
>> remove_percpu_irq(), unused, is also removed.
>
> Do you feel that this series adds value ?, if not, i will abandon this
> series.

 7 files changed, 10 insertions(+), 75 deletions(-)

is definitely worth it. There is no point in having two interfaces. I'll
have a look at the changes later today.

Thanks,

        tglx