[PATCH] genirq: Pass desc to __irq_free instead of irq number

[PATCH] genirq: Pass desc to __irq_free instead of irq number

[Uwe Kleine-König]

Given that irq_to_desc() is a radix_tree_lookup and the reverse
operation is only a pointer dereference and that all callers of
__free_irq already have the desc, pass the desc instead of the irq
number.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this is a micro optimisation I noticed while looking at the irq code.

Best regards
Uwe

 kernel/irq/manage.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0f922729bab9..a08ba9a8da14 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1519,9 +1519,9 @@ EXPORT_SYMBOL_GPL(setup_irq);
  * Internal function to unregister an irqaction - used to free
  * regular and special interrupts that are part of the architecture.
  */
-static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
+static struct irqaction *__free_irq(struct irq_desc *desc, void *dev_id)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	unsigned irq = desc->irq_data.irq;
 	struct irqaction *action, **action_ptr;
 	unsigned long flags;
 
@@ -1651,7 +1651,7 @@ void remove_irq(unsigned int irq, struct irqaction *act)
 	struct irq_desc *desc = irq_to_desc(irq);
 
 	if (desc && !WARN_ON(irq_settings_is_per_cpu_devid(desc)))
-		__free_irq(irq, act->dev_id);
+		__free_irq(desc, act->dev_id);
 }
 EXPORT_SYMBOL_GPL(remove_irq);
 
@@ -1685,7 +1685,7 @@ const void *free_irq(unsigned int irq, void *dev_id)
 		desc->affinity_notify = NULL;
 #endif
 
-	action = __free_irq(irq, dev_id);
+	action = __free_irq(desc, dev_id);
 
 	if (!action)
 		return NULL;
-- 
2.16.2

Re: [PATCH] genirq: Pass desc to __irq_free instead of irq number

[Uwe Kleine-König]

Hello,

On Mon, Mar 19, 2018 at 11:52:02AM +0100, Uwe Kleine-König wrote:
> Given that irq_to_desc() is a radix_tree_lookup and the reverse
> operation is only a pointer dereference and that all callers of
> __free_irq already have the desc, pass the desc instead of the irq
> number.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> this is a micro optimisation I noticed while looking at the irq code.

This made it into next as 83ac4ca943affce56a2c408ddb2f5232f478fb11 and
ba63053f018721a0ad60b083e21a15e324a4a8c4. Is it right that it is there
twice? And in the Author line the - in my name is missing. Would be
great if that could be fixed up.

Something like:

	git filter-branch -f --env-filter 'test "$GIT_AUTHOR_NAME" != "Uwe Kleine König" || GIT_AUTHOR_NAME="Uwe Kleine-König"' linus/master..

should do the trick.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] genirq: Pass desc to __irq_free instead of irq number

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Mon, 26 Mar 2018, Uwe Kleine-König wrote:
> 
> On Mon, Mar 19, 2018 at 11:52:02AM +0100, Uwe Kleine-König wrote:
> > Given that irq_to_desc() is a radix_tree_lookup and the reverse
> > operation is only a pointer dereference and that all callers of
> > __free_irq already have the desc, pass the desc instead of the irq
> > number.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > this is a micro optimisation I noticed while looking at the irq code.
> 
> This made it into next as 83ac4ca943affce56a2c408ddb2f5232f478fb11 and
> ba63053f018721a0ad60b083e21a15e324a4a8c4. Is it right that it is there
> twice?

No. It's only one instance.

> And in the Author line the - in my name is missing. Would be great if
> that could be fixed up.

Sorry, I'm surely not rebasing that branch just to add a missing dash.

Thanks,

	tglx

Re: [PATCH] genirq: Pass desc to __irq_free instead of irq number

[Uwe Kleine-König]

Hello Thomas,

On Mon, Mar 26, 2018 at 11:43:27AM +0200, Thomas Gleixner wrote:
> On Mon, 26 Mar 2018, Uwe Kleine-König wrote:
> > 
> > On Mon, Mar 19, 2018 at 11:52:02AM +0100, Uwe Kleine-König wrote:
> > > Given that irq_to_desc() is a radix_tree_lookup and the reverse
> > > operation is only a pointer dereference and that all callers of
> > > __free_irq already have the desc, pass the desc instead of the irq
> > > number.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > > 
> > > this is a micro optimisation I noticed while looking at the irq code.
> > 
> > This made it into next as 83ac4ca943affce56a2c408ddb2f5232f478fb11 and
> > ba63053f018721a0ad60b083e21a15e324a4a8c4. Is it right that it is there
> > twice?
> 
> No. It's only one instance.

$ git log --oneline linus/master..next/master --grep='genirq: Pass desc to __irq_free'
83ac4ca943af genirq: Pass desc to __irq_free instead of irq number
ba63053f0187 genirq: Pass desc to __irq_free instead of irq number

$ git show 83ac4ca943af ba63053f0187 | git patch-id
ee0091f074eb45677b190f29e30ab97adad449d9 83ac4ca943affce56a2c408ddb2f5232f478fb11
ee0091f074eb45677b190f29e30ab97adad449d9 ba63053f018721a0ad60b083e21a15e324a4a8c4

They both come into next by commit c5b27fb35b1a ("Merge remote-tracking branch
'tip/auto-latest'"):

$ git rev-parse c5b27fb35b1a^2^2~4
83ac4ca943affce56a2c408ddb2f5232f478fb11
$ git rev-parse c5b27fb35b1a^2~25^2
ba63053f018721a0ad60b083e21a15e324a4a8c4

You still claim there is only one instance, I assume that's because the branch
that is pulled into next (tip/auto-latest) is autogenerated and contains some
cruft?

> > And in the Author line the - in my name is missing. Would be great if
> > that could be fixed up.
> 
> Sorry, I'm surely not rebasing that branch just to add a missing dash.

:-| I wonder how this error happend. Is there a script to fix? If so I'd
like to take a look.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] genirq: Pass desc to __irq_free instead of irq number

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 779 bytes --]

On Mon, 26 Mar 2018, Uwe Kleine-König wrote:
> On Mon, Mar 26, 2018 at 11:43:27AM +0200, Thomas Gleixner wrote:
> > On Mon, 26 Mar 2018, Uwe Kleine-König wrote:
> You still claim there is only one instance, I assume that's because the branch
> that is pulled into next (tip/auto-latest) is autogenerated and contains some
> cruft?

The branch is autogenerated and reset on top of Linus head on a regular
base and that's what I looked at. It does not contain any cruft at all. It
contains a single instance of that patch, i.e. 83ac4ca943af

The irq/core branch was rebased once and the commit in question was
cherry-picked so it ended up with a different sha1. It probably was in next
with the old sha1 for a day or two, but it's definitely not in
tip/auto-latest.

Thanks,

	tglx