lkml.org 
[lkml]   [2012]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/2] irqdomain: Fix up linear revmap for non-zero hwirq displacement.
Date
On Thu, 21 Jun 2012 10:19:40 +0900, Paul Mundt <lethal@linux-sh.org> wrote:
> On Sat, Jun 16, 2012 at 08:14:19AM +0900, Paul Mundt wrote:
> > On Fri, Jun 15, 2012 at 12:25:40PM -0600, Grant Likely wrote:
> > > On Wed, 13 Jun 2012 16:34:00 +0900, Paul Mundt <lethal@linux-sh.org> wrote:
> > > > Presently the linear revmap code assumes that all hwirqs start at 0,
> > > > using the hwirq directly as an index value for the lookup. In the case of
> > > > legacy revmaps this isn't necessarily the case, as the first_hwirq value
> > > > passed in can be non-zero, causing those types of users to silently have
> > > > their IRQs placed in the radix tree instead.
> > > >
> > > > With this change, hwirq displacement is factored in at association time
> > > > directly. This also makes it possible for non-legacy users to use linear
> > > > revmaps regardless of hwirq base position. This could potentially lead to
> > > > a bug if there's an attempt to associate multiple times in to the linear
> > > > map in a nonsensical and non-linear order, but at that point being
> > > > silently punted to the radix tree is likely to be the least of your
> > > > concerns (in such a case it's fairly trivial to simply extend
> > > > irq_domain_add_linear() to take a hwirq base and move the linear base
> > > > assignment there).
> > >
> > > I actually hoped to be rid of the whole hwirq start offset thing.
> > > Doing without it simplifies the code, is slightly faster. I suspect
> > > very few controllers actually need it, and for those that do I'm
> > > hoping the wasted space is in the order of 0-32 words.
> > >
> > > Instead of this, can we change the affected controllers to use the
> > > maximum hwirq number when setting the size of the linear map?
> > >
> > > Do you have hardware where the first hwirq is a >32 number?
> > >
> > Yes. On the CPU I was just working on I have two linear ranges and a
> > tree, one of the linear ranges begins at hwirq 56. On other CPUs we have
> > linear ranges that begin at 64, 72, etc. most of which are fairly low in
> > the space. On newer parts on the ARM side there are also controllers with
> > ranges that begin > 400.
> >
> > I don't particularly care for the linear_start hack myself either, but I
> > couldn't think of any cleaner approach for it. The simplest might be if
> > we can just bury these details in a domain-specific canonicalization op
> > (distinctly different from xlate), and plug it in for the few cases that
> > need a non-zero hwirq base. I don't mind hacking that up if you're more
> > agreeable to that approach.
>
> Ping?
>
> We can't do away with the first_irq thing in the legacy->linear merge
> without at least having a strategy for getting existing users off of it.
> Requiring the linear revmap to always begin at 0 seems like a significant
> regression in functionality for marginal performance gain, so if you're
> not willing to have the linear_start factored in we do need some other
> alternative. I've proposed several, if you don't like any of those you
> are welcome to propose an alternative. I don't mind doing the work one
> way or the other, but I do mind losing the functionality.

From another perspective, even if irqs do start at 400, that is wasted
space of 1600 bytes. Less than half a page. It still isn't a huge
amount. The choice to use a linear vs. radix map is a choice between
speed and sparse flexability. Considering that one of Ben's concerns is
preserving the fastest lookup path possible, I greatly prefer the
simplicity of a single offset between hwirq and irq. If you want to
avoid it in your driver, I won't object, but it seems to me a case of
premature optimization.

However, I do agree that allocating 400 unused irq_descs would be a
problem, but the patches don't work that way. irq_domain_add_legacy()
only calls irq_domain_associate_many() on the requested range of hwirqs
by using the value of hwirq_base passed in.

> Punishing legacy users with leading gaps in their revmap is likewise
> undesirable, especially as that most of the in-tree users (regmap-irq
> especially) of this functionality are using this legacy behaviour without
> incident.

Hardly punishment. It is a different optimization decision. The vast
majority of _add_legacy calls use 0 for hwirq_base, and the ones that
don't use a very small number.

...ummm are we talking about the same things? You mention regmap-irq
specifically, but regmap irq also uses 0 for hwirq_start, so there is no
leading gap.

g.


--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.


\
 
 \ /
  Last update: 2012-07-12 06:21    [W:2.378 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site