LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Arjan van de Ven <arjan@infradead.org>,
	Ingo Molnar <mingo@elte.hu>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andi Kleen <ak@suse.de>, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC] killing the NR_IRQS arrays.
Date: Tue, 27 Feb 2007 13:29:11 -0700	[thread overview]
Message-ID: <m1abyzl4yw.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <1171844753.5644.174.camel@localhost.localdomain> (Benjamin Herrenschmidt's message of "Mon, 19 Feb 2007 11:25:53 +1100")


A quick update.  I did some work on this and have some observations.

- Every back end irq implementation seems to have a different name for
  the structure that describes irqs.  So picking struct irq which is
  different from everything seems to make sense.  At the very least
  a empty struct irq can be embedded in the architecture specific
  irq description structure and we can use container_of to get back
  to it.

- The name struct irq conflicts with a spurious definition in
  include/pcmcia/cs.h, but otherwise is fine.  So it probably makes
  sense to use.

- Updating all of the drivers is going to be a pain for precisely the
  reason we need to update the drivers.  irq numbers are not handled
  at all cleanly.  In attempting just a few conversions I have seen
  irq number stashed in everything for a u8 to a u64.  I have seen
  all values <= 0 thrown out as invalid.

  So we have a decade or more of accumulated inconsistencies and it is
  getting painful to work with.  Changing the type to something that
  won't support all of the old hacks should help cleanup the code.

- Because drivers are not consistent in their handling of irq numbers 
  whatever path I take to a conversion each patch needs to be thought
  about instead of just performed.  So I'm going to have to double
  the API so a gradual conversion is possible. 

- Converting the genirq code to use struct irq_desc pointers
  throughout (instead of unsigned int irq) is straight forward and
  mindless.  Though it is tedious.  Like I expected the drivers to be
  :(

  So it looks like all I need to do to convert the genirq backend is
  to break the work up into small enough patches that each patch is
  obviously correct.  Then compiling on the different architectures
  can just serve as a spot check, not as absolutely required step
  during the code conversion.

- The converted genirq code was short and easier to follow.  Mostly
  because I got to kill all of the if (irq >= NR_IRQS) tests...
  Null pointer dereferences are your friends!

- The are only 3 or 4 arrays of size NR_IRQS in non-architecture code
  and I have patches in my queue to kill them, so that isn't too bad.

- All of the drivers that handle irqs need to be touched because one
  of the parameters to the irq handler is the interrupt number.  So
  that needs to be converted.  



So I think the path should be:

* Kill the arrays of size NR_IRQS in non-arch code.

* Add a variation of the API in interrupt.h that uses
  "struct irq *irq" instead of "unsigned int irq"
  
  Probably replacing request_irq with irq_request or something
  trivial like that.

  This will need to touch all of different irq implementation back
  ends, but only very lightly.

* Convert the generic irq code to use struct irq * everywhere it
  current uses "unsigned int irq".

* Start on the conversions of drivers and subsystems picking on
  the easy ones first :)

* Adding for_each_irq_desc() and similar helpers to the generic
  irq code.

* Add support in the generic irq code for architectures that don't
  have a giant array of irqs.  

* Convert x86_64 and i386 to dynamically allocate their irqs.
  Routines using the old interfaces will be no longer O(1) more
  likely O(N). So will be slow but request_irq and free_irq are
  no where near the fast path so it doesn't matter.  enable_irq
  and disable_irq are the only cases that might matter and they
  occur rarely enough fixing the drivers that matter should not
  be a problem.

* Ultimately finish converting all of the drivers and remove the
  compatibility cruft.

I will look at getting things started and some patches into -mm
sometime next month.

Eric



  reply	other threads:[~2007-02-27 20:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-16 12:10 [RFC] killing the NR_IRQS arrays Eric W. Biederman
2007-02-16 12:16 ` Andi Kleen
2007-02-16 15:35   ` Eric W. Biederman
2007-02-16 12:41 ` Ingo Molnar
2007-02-16 15:23   ` Eric W. Biederman
2007-02-16 15:49   ` Eric W. Biederman
2007-02-16 22:33   ` Benjamin Herrenschmidt
2007-02-18 21:24   ` Arjan van de Ven
2007-02-19  0:25     ` Benjamin Herrenschmidt
2007-02-27 20:29       ` Eric W. Biederman [this message]
2007-02-28  0:41         ` Arnd Bergmann
2007-02-28  7:20           ` Eric W. Biederman
2007-02-28  8:09             ` Benjamin Herrenschmidt
2007-02-28 13:28               ` Eric W. Biederman
2007-02-28 12:24             ` Arnd Bergmann
2007-02-28 13:02               ` Segher Boessenkool
2007-02-28 13:53               ` Eric W. Biederman
2007-03-01 10:47                 ` Benjamin Herrenschmidt
2007-02-16 18:07 ` Jeremy Fitzhardinge
2007-02-16 19:01   ` Eric W. Biederman
2007-02-16 19:06     ` Jeremy Fitzhardinge
2007-02-16 19:45 ` Arnd Bergmann
2007-02-16 19:52   ` Russell King
2007-02-16 20:43     ` Arnd Bergmann
2007-02-16 20:59       ` Russell King
2007-02-16 22:37     ` Benjamin Herrenschmidt
2007-02-17  1:37       ` Arnd Bergmann
2007-02-17  4:00         ` Benjamin Herrenschmidt
2007-02-17  9:06           ` Eric W. Biederman
2007-02-17 21:15             ` Benjamin Herrenschmidt
2007-02-18  6:30               ` Eric W. Biederman
2007-02-18 20:01                 ` Benjamin Herrenschmidt
2007-02-17  9:34     ` Eric W. Biederman
2007-02-17 21:20       ` Benjamin Herrenschmidt
2007-02-18  3:58         ` Eric W. Biederman
2007-02-16 22:29 ` Benjamin Herrenschmidt
2007-02-17  8:51   ` Eric W. Biederman
2007-02-17 21:04     ` Benjamin Herrenschmidt
2007-02-18  4:58       ` Eric W. Biederman
2007-02-18 19:58         ` Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1abyzl4yw.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=arjan@infradead.org \
    --cc=benh@kernel.crashing.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).