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:4.094 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site