[kernel,v4,5/8] genirq: Add free_irq hook for IRQ descriptor and use for mapping disposal

We want to make the irq_desc.kobj's release hook free associated resources
but we do not want to pollute the irqdesc code with domains.

This adds a free_irq hook which is called when the last reference to
the descriptor is dropped.

The first user is mapped irqs. This potentially can break the existing
users; however they seem to do the right thing and call dispose once
per mapping.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 include/linux/irqdesc.h    |  1 +
 include/linux/irqdomain.h  |  2 --
 include/linux/irqhandler.h |  1 +
 kernel/irq/irqdesc.c       |  3 +++
 kernel/irq/irqdomain.c     | 14 ++++++++++++--
 5 files changed, 17 insertions(+), 4 deletions(-)

Alexey,

On Tue, Nov 24 2020 at 17:17, Alexey Kardashevskiy wrote:
> We want to make the irq_desc.kobj's release hook free associated resources

> but we do not want to pollute the irqdesc code with domains.


Can you please describe your changelog in factual ways without 'we and I
and want'? See Documentation/process/

> This adds a free_irq hook which is called when the last reference to

> the descriptor is dropped.

>

> The first user is mapped irqs. This potentially can break the existing

> users; however they seem to do the right thing and call dispose once

> per mapping.


Q: How is this supposed to work with CONFIG_SPARSE_IRQ=n?
A: Not at all.

Also 'seem to do the right thing' is from the same quality as 'should
not break stuff'. Either you have done a proper analysis or you did
not. Aside of that how is anyone supposed to decode the subtle wreckage
which is this going to cause if 'seem to do the right thing' turns out
to be wrong?

> -void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)

> +static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)

>  {

>  	struct irq_data *irq_data = irq_get_irq_data(irq);

>  	irq_hw_number_t hwirq;

> @@ -582,6 +582,13 @@ void irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,

>  }

>  EXPORT_SYMBOL_GPL(irq_domain_associate_many);

>  

> +static void irq_mapped_free_desc(struct irq_desc *desc)


That function name is really misleading and badly chosen. The function
is not about freeing the irq descriptor as the name suggests. It's
called from that code in order to clean up the mapping.

> +{

> +	unsigned int virq = desc->irq_data.irq;

> +

> +	irq_domain_disassociate(desc->irq_data.domain, virq);

> +}

> +

>  /**

>   * irq_create_direct_mapping() - Allocate an irq for direct mapping

>   * @domain: domain to allocate the irq for or NULL for default domain

> @@ -638,6 +645,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,

>  {

>  	struct device_node *of_node;

>  	int virq;

> +	struct irq_desc *desc;


This code uses reverse fir tree ordering of variables

  	struct device_node *of_node;
	struct irq_desc *desc;
  	int virq;

Why? Because it's simpler to read than the vertical camel case above.

Thanks,

        tglx

On Mon, Nov 30 2020 at 23:18, Thomas Gleixner wrote:
> On Tue, Nov 24 2020 at 17:17, Alexey Kardashevskiy wrote:

>> We want to make the irq_desc.kobj's release hook free associated resources

>> but we do not want to pollute the irqdesc code with domains.

>

> Can you please describe your changelog in factual ways without 'we and I

> and want'? See Documentation/process/


And while we are at process. MAINTAINERS clearly says:

IRQ DOMAINS (IRQ NUMBER MAPPING LIBRARY)
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

IRQ SUBSYSTEM
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

So why are these patches not applying against that git branch?

Thanks,

        tglx