lkml.org 
[lkml]   [2012]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions
From
Date
On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> In preparation to remove the slow revmap path, eliminate the public
> radix revmap lookup functions. This simplifies the code and makes the
> slowpath removal patch a lot simpler.

The idea here was to avoid a test (performance in the fast path) since
the controller knows the type of revmap it uses. You could just remove
the fallback to the slow path if there's a mismatch and keep the WARN_ON
no ? No biggie, it's just a switch/case with fairly predictable outcome
I suppose ...

A few other nits:

> return irq;
> diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
> index cd1d18d..9049d9f 100644
> --- a/arch/powerpc/sysdev/xics/xics-common.c
> +++ b/arch/powerpc/sysdev/xics/xics-common.c
> @@ -329,9 +329,6 @@ static int xics_host_map(struct irq_domain *h, unsigned int virq,
>
> pr_devel("xics: map virq %d, hwirq 0x%lx\n", virq, hw);
>
> - /* Insert the interrupt mapping into the radix tree for fast lookup */
> - irq_radix_revmap_insert(xics_host, virq, hw);
> -
> /* They aren't all level sensitive but we just don't really know */
> irq_set_status_flags(virq, IRQ_LEVEL);

This looks like it belongs in a different patch, possibly "[02/12]
irqdomain: Always update revmap when setting up a virq" no ?

> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 3425631..d8b88c5 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -169,10 +169,6 @@ static inline int irq_create_identity_mapping(struct irq_domain *host,
> return irq_create_strict_mappings(host, hwirq, hwirq, 1);
> }
>
> -extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
> - irq_hw_number_t hwirq);
> -extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
> - irq_hw_number_t hwirq);
> extern unsigned int irq_linear_revmap(struct irq_domain *host,
> irq_hw_number_t hwirq);
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 1a8f3d2..79a7711 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -415,7 +415,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
> domain->revmap_data.linear.revmap[hwirq] = virq;
> break;
> case IRQ_DOMAIN_MAP_TREE:
> - irq_radix_revmap_insert(domain, virq, hwirq);
> + mutex_lock(&revmap_trees_mutex);
> + radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> + mutex_unlock(&revmap_trees_mutex);
> break;
> }

That too looks like it belongs in another patch.

> @@ -688,64 +690,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
> EXPORT_SYMBOL_GPL(irq_find_mapping);
>
> /**
> - * irq_radix_revmap_lookup() - Find a linux irq from a hw irq number.
> - * @domain: domain owning this hardware interrupt
> - * @hwirq: hardware irq number in that domain space
> - *
> - * This is a fast path, for use by irq controller code that uses radix tree
> - * revmaps
> - */
> -unsigned int irq_radix_revmap_lookup(struct irq_domain *domain,
> - irq_hw_number_t hwirq)
> -{
> - struct irq_data *irq_data;
> -
> - if (WARN_ON_ONCE(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
> - return irq_find_mapping(domain, hwirq);
> -
> - /*
> - * Freeing an irq can delete nodes along the path to
> - * do the lookup via call_rcu.
> - */
> - rcu_read_lock();
> - irq_data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
> - rcu_read_unlock();
> -
> - /*
> - * If found in radix tree, then fine.
> - * Else fallback to linear lookup - this should not happen in practice
> - * as it means that we failed to insert the node in the radix tree.
> - */
> - return irq_data ? irq_data->irq : irq_find_mapping(domain, hwirq);
> -}
> -EXPORT_SYMBOL_GPL(irq_radix_revmap_lookup);
> -
> -/**
> - * irq_radix_revmap_insert() - Insert a hw irq to linux irq number mapping.
> - * @domain: domain owning this hardware interrupt
> - * @virq: linux irq number
> - * @hwirq: hardware irq number in that domain space
> - *
> - * This is for use by irq controllers that use a radix tree reverse
> - * mapping for fast lookup.
> - */
> -void irq_radix_revmap_insert(struct irq_domain *domain, unsigned int virq,
> - irq_hw_number_t hwirq)
> -{
> - struct irq_data *irq_data = irq_get_irq_data(virq);
> -
> - if (WARN_ON(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
> - return;
> -
> - if (virq) {
> - mutex_lock(&revmap_trees_mutex);
> - radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> - mutex_unlock(&revmap_trees_mutex);
> - }
> -}
> -EXPORT_SYMBOL_GPL(irq_radix_revmap_insert);
> -
> -/**
> * irq_linear_revmap() - Find a linux irq from a hw irq number.
> * @domain: domain owning this hardware interrupt
> * @hwirq: hardware irq number in that domain space

Cheers,
Ben.




\
 
 \ /
  Last update: 2012-06-16 08:41    [W:0.148 / U:2.768 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site