public inbox for linux-arch@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 00/75] genirq: Overhaul for 2.6.39
@ 2011-02-10 23:35 Thomas Gleixner
  2011-02-10 23:53 ` Linus Torvalds
  2011-02-11  4:03 ` Frank Rowand
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-10 23:35 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, linux-arch, Linus Torvals,
	Greg Kroah-Hartman

This is a major overhaul of the generic interrupt layer.

     - Namespace cleanup

     - Further encapsulation of the core state

     - Spurious/Poll handling fixes

     - Stop setaffinity blindly manipulating affinity mask

     - Cleanups and enhancements all over the place

1) Namespace cleanup

   I reworked the slightly confusing (sorry, my bad) namespace of the
   various accessor functions. They follow a simple scheme now:

   irq_set/get_xxx(unsigned int irq, ...)
   irq_data_set/get_xxx(struct irq_data *d, ...)

   That's the first patch in the series and I'm going to apply it to
   a separate rc-4 based branch which everybody can pull in to
   do the changes now in his tree.

2) Encapsulation

   After __do_IRQ() was finally gone (Thanks to all involved!) I
   wanted to restructure the core status handling and cleanup the
   IRQ_* flag space.

   A quick grep over the tree made me really shudder, so I converted
   13 archs which are known to follow core changes slow to the new
   irq_chip callback functions and cleaned up the irq_desc accessors
   while at it. That way I learned also a lot about arch requirements.

   Then I looked at the remaining places which deal with
   irq_desc->status directly.

   1) Special EOI handling in sparc64/mips/sh. Easy to replace.

   2) Random abusers of chained_interrupt_handler. The worst offender
      is arch/powerpc/sysdev/fsl_msi.c, which implements a combo
      eoi/level flow handler instead of using the existing ones.

   3) Random places which quirk around their chip oddities instead of
      looking for solutions which can be made generic. One of them is
      the requirement to mask irq chips before setting the trigger
      type. Why is nobody talking to me ?

   4) Random leftovers from the __do_IRQ() semantics which are easy to
      cleanup.

   5) Initializations with IRQ_DISABLED and random IRQ_ flags.
      IRQ_DISABLED is core internal state and already set by default.
      The settable flags have to use the provided modifier functions.

   6) Weird flow handlers in m68knommu/arm/powerpc. All of them can
      be either replaced by existing handlers or can use an existing
      handler with minimal modifications. Patches are already out.

   7) Totally broken debug code in gpiolib and a copy of the same in
      some arm Soc code. Patch is in this series to kill it. Also
      remove unused code in arm/mach-tegra which seems to be some
      relict from the android mess.

   So there is no real requirement to expose the guts of the generic
   irq code to the world.

   Even if all is cleaned up, I'm tired of:

   	- Having to chase the creative abusage all the time

	- People not talking to me about their special requirements
	
	- Spending another day to find out that someone had typoed
	  desc->status |= IRQ_LEVEL; into desc->status = IRQ_LEVEL;
	  in some weird arch code and then complaining about the
	  core code playing silly buggers.

   So I decided to encapsulate the state into the core and make it
   hard to access and offenders easy to find. For now I keep the
   existing and used IRQ_* status bits up to date, unless
   GENERIC_HARDIRQS_NO_COMPAT=y which can be selected by the
   architecture. Be aware that not setting that config adds extra
   overhead to the core code.

   For all real requirements which I found, I provided solutions
   either in the core itself or information accessible via a state
   field in irq_data, which is handed to the irq_chip functions. So
   setting GENERIC_HARDIRQS_NO_COMPAT=y should be not a big deal on
   most architectures. There are some gpio and mfd drivers which need
   to be converted to threaded interrupts instead of having the old
   style racy and ugly disable_irq_nosync/schedule_work()
   implementation.

   There are a few places which need some thought like the powerpc
   kexec shutdown, but even that stuff should be generic as it makes
   sense to cleanup irqs before booting into a new kernel. Nothing
   which can't be solved though.

   Note that most accessors to irq_desc are going away sooner than
   later and long term I want to remove irq_desc from the public space
   completely.

   One step to this is a generic show_interrupts() function, which
   replaces the 23 copies with random modifications and different bugs
   inside. Unless you switch over you will be reminded by a nice
   deprecated warning.

3) Spurious/Poll 

   This is a long neglected piece of code and got an overhaul so it's
   simpler and less broken than it used to be.

4) irq_setaffinity()

   Does not longer blindly update the affinity mask before calling
   into the arch code. Also the chip functions can now return a 
   return code, which tells the core that it modified the mask
   itself. That's useful when arch code restricts the mask further
   than the core code can do.

5) Cleanups and enhancements

   Main cleanup happened in poll and the consolidation of the flow
   handlers which moved their duplicated code into handle_irq_event
   
   irq_set_type() now checks chip.flags for a chip flag, which indicates
   that the chip needs to be masked before setting the trigger type.
   That avoids duplicated code and fiddling with internal core state.

If no major outcry comes I'm going to push this into tip/irq/core and
next.

A preview is available at:

git://git.kernel.org/pub/scm/linux/kernel/git/tglx/linux-2.6-genirq.git irq/core

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-10 23:35 [patch 00/75] genirq: Overhaul for 2.6.39 Thomas Gleixner
@ 2011-02-10 23:53 ` Linus Torvalds
  2011-02-11  0:00   ` Thomas Gleixner
  2011-02-11  4:03 ` Frank Rowand
  1 sibling, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2011-02-10 23:53 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Greg Kroah-Hartman

On Thu, Feb 10, 2011 at 3:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> A preview is available at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/linux-2.6-genirq.git irq/core

For things like this, please just always include a diffstat so that
people can tell from the email whether they care or not.

                  Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-10 23:53 ` Linus Torvalds
@ 2011-02-11  0:00   ` Thomas Gleixner
  2011-02-11  0:28     ` Linus Torvalds
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-11  0:00 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Greg Kroah-Hartman

On Thu, 10 Feb 2011, Linus Torvalds wrote:

> On Thu, Feb 10, 2011 at 3:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > A preview is available at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/linux-2.6-genirq.git irq/core
> 
> For things like this, please just always include a diffstat so that
> people can tell from the email whether they care or not.

Oops. Forgot.

 arch/arm/mach-ep93xx/gpio.c         |   38 ---
 arch/arm/mach-ns9xxx/irq.c          |   58 -----
 arch/arm/mach-tegra/gpio.c          |   63 ------
 arch/arm/mach-tegra/irq.c           |   58 -----
 arch/m68knommu/platform/5272/intc.c |    7 
 arch/x86/Kconfig                    |    3 
 arch/x86/kernel/apic/io_apic.c      |   55 ++---
 arch/x86/kernel/hpet.c              |    2 
 arch/x86/kernel/irq.c               |   63 ------
 drivers/gpio/gpiolib.c              |   44 ----
 fs/proc/interrupts.c                |    2 
 include/linux/interrupt.h           |   22 --
 include/linux/irq.h                 |  314 ++++++++++++++++++++++++++------
 include/linux/irqdesc.h             |   68 ++++++
 kernel/irq/Kconfig                  |   10 -
 kernel/irq/autoprobe.c              |   48 ++--
 kernel/irq/chip.c                   |  353 +++++++++++++++++-------------------
 kernel/irq/compat.h                 |   72 +++++++
 kernel/irq/debug.h                  |   40 ++++
 kernel/irq/handle.c                 |   55 ++++-
 kernel/irq/internals.h              |  136 +++++++++----
 kernel/irq/irqdesc.c                |    6 
 kernel/irq/manage.c                 |  303 ++++++++++++++++++------------
 kernel/irq/migration.c              |   38 ++-
 kernel/irq/pm.c                     |    9 
 kernel/irq/proc.c                   |   73 +++++++
 kernel/irq/resend.c                 |   17 -
 kernel/irq/settings.h               |  131 +++++++++++++
 kernel/irq/spurious.c               |  164 ++++++++++------
 29 files changed, 1353 insertions(+), 899 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11  0:00   ` Thomas Gleixner
@ 2011-02-11  0:28     ` Linus Torvalds
  2011-02-11  0:49       ` Thomas Gleixner
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2011-02-11  0:28 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Greg Kroah-Hartman

On Thu, Feb 10, 2011 at 4:00 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
>  29 files changed, 1353 insertions(+), 899 deletions(-)

So what is it that adds so many lines? Your description made me think
"cleanups", not "50% more code"

(Yeah, yeah, I could look at the code, but I also want to point out
that this really doesn't look like much of an improvement from a
high-level standpoint)

                             Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11  0:28     ` Linus Torvalds
@ 2011-02-11  0:49       ` Thomas Gleixner
  2011-02-11 13:05         ` Thomas Gleixner
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-11  0:49 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Greg Kroah-Hartman

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1212 bytes --]

On Thu, 10 Feb 2011, Linus Torvalds wrote:

> On Thu, Feb 10, 2011 at 4:00 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> >  29 files changed, 1353 insertions(+), 899 deletions(-)
> 
> So what is it that adds so many lines? Your description made me think
> "cleanups", not "50% more code"
> 
> (Yeah, yeah, I could look at the code, but I also want to point out
> that this really doesn't look like much of an improvement from a
> high-level standpoint)

Part of it is then namespace cleanup which makes the deprecated
functions wrappers around the new ones. Easy to solve with a script.

More stuff will go way again when the wrappers which I added to
prevent wreckage of arch/* fiddling with irq_desc are gone

The other things are simple inline functions which provide accessors
to state which is intentionally named:

   state_use_accessors, 

For simple reasons:

1) while you type it it should click that you're doing something wrong

2) easy to grep for to find offenderrs. git grep status sucks

There are more wrappers in the core code kernel/irq solely to prevent
using anything which got deprecated.

So yes, it's net more source lines, but not resulting in any binary
bloat.

Thanks,

	tglx



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-10 23:35 [patch 00/75] genirq: Overhaul for 2.6.39 Thomas Gleixner
  2011-02-10 23:53 ` Linus Torvalds
@ 2011-02-11  4:03 ` Frank Rowand
  1 sibling, 0 replies; 12+ messages in thread
From: Frank Rowand @ 2011-02-11  4:03 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Linus Torvals,
	Greg Kroah-Hartman

On 02/10/11 15:35, Thomas Gleixner wrote:
> This is a major overhaul of the generic interrupt layer.
> 
>      - Namespace cleanup
> 
>      - Further encapsulation of the core state
> 
>      - Spurious/Poll handling fixes
> 
>      - Stop setaffinity blindly manipulating affinity mask
> 
>      - Cleanups and enhancements all over the place

Hi Thomas,

You seem to have overlooked a patch from me:

Update comments to match code change in 70aedd24
The comments for enable_irq() were updated correctly, but disable_irq_nosync()
and disable_irq() were missed.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>

---
 kernel/irq/manage.c |    7 	5 +	2 -	0 !
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: b/kernel/irq/manage.c
===================================================================
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -353,7 +353,8 @@ void __disable_irq(struct irq_desc *desc
  *	Unlike disable_irq(), this function does not ensure existing
  *	instances of the IRQ handler have completed before returning.
  *
- *	This function may be called from IRQ context.
+ *	This function may be called from IRQ context only when
+ *	desc->chip->bus_lock and desc->chip->bus_sync_unlock are NULL !
  */
 void disable_irq_nosync(unsigned int irq)
 {
@@ -381,7 +382,9 @@ EXPORT_SYMBOL(disable_irq_nosync);
  *	to complete before returning. If you use this function while
  *	holding a resource the IRQ handler may need you will deadlock.
  *
- *	This function may be called - with care - from IRQ context.
+ *	This function may be called - with care - from IRQ context only when
+ *	desc->chip->bus_lock and desc->chip->bus_sync_unlock are NULL !
+ *	See synchronize_irq() comments for explanation of "with care".
  */
 void disable_irq(unsigned int irq)
 {

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11  0:49       ` Thomas Gleixner
@ 2011-02-11 13:05         ` Thomas Gleixner
  2011-02-11 13:59           ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-11 13:05 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Peter Zijlstra, linux-arch, Greg Kroah-Hartman

B1;2401;0cOn Fri, 11 Feb 2011, Thomas Gleixner wrote:
> On Thu, 10 Feb 2011, Linus Torvalds wrote:
> So yes, it's net more source lines, but not resulting in any binary
> bloat.

Just checked. When the compat layer goes away it will kill about 500
lines. So it's less code with better encapsulation.

Once all genirq archs convert to the generic irq_show_interrupts(),
this will kill another 1000+ lines.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11 13:05         ` Thomas Gleixner
@ 2011-02-11 13:59           ` Ingo Molnar
  2011-02-11 14:26             ` Thomas Gleixner
  0 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2011-02-11 13:59 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Torvalds, LKML, Peter Zijlstra, linux-arch,
	Greg Kroah-Hartman


* Thomas Gleixner <tglx@linutronix.de> wrote:

> B1;2401;0cOn Fri, 11 Feb 2011, Thomas Gleixner wrote:
> > On Thu, 10 Feb 2011, Linus Torvalds wrote:
> > So yes, it's net more source lines, but not resulting in any binary
> > bloat.
> 
> Just checked. When the compat layer goes away it will kill about 500
> lines. So it's less code with better encapsulation.
> 
> Once all genirq archs convert to the generic irq_show_interrupts(),
> this will kill another 1000+ lines.

So while this is the first step:

>  29 files changed, 1353 insertions(+), 899 deletions(-)

It turns into this end result (mockup):

>  129 files changed, 1353 insertions(+), 2400 deletions(-)

Right? Or, more likely, considering all the surrounding code changes, something 
like:

>  129 files changed, 4353 insertions(+), 5400 deletions(-)

Did I get the file count right - roughtly how many files are affected throughout all 
architectures? The changes are massively intrusive and widely spread out so we 
cannot do them in one go, right?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11 13:59           ` Ingo Molnar
@ 2011-02-11 14:26             ` Thomas Gleixner
  2011-02-11 14:26               ` Thomas Gleixner
  2011-02-13 12:50               ` Sam Ravnborg
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-11 14:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, LKML, Peter Zijlstra, linux-arch,
	Greg Kroah-Hartman

[-- Attachment #1: Type: TEXT/PLAIN, Size: 661 bytes --]

On Fri, 11 Feb 2011, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
> >  129 files changed, 4353 insertions(+), 5400 deletions(-)
> 
> Did I get the file count right - roughtly how many files are affected throughout all 
> architectures? The changes are massively intrusive and widely spread out so we 
> cannot do them in one go, right?

Alone the stuff which fiddles in irq_desc->status for whatever reasons
is >100 files in arch/. And that has do be done manually case by case.

The namespace cleanup affects about 300 files in arch, but that can be
done in one go right after -rc1 if I manage to get the regex straight :)

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11 14:26             ` Thomas Gleixner
@ 2011-02-11 14:26               ` Thomas Gleixner
  2011-02-13 12:50               ` Sam Ravnborg
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-11 14:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, LKML, Peter Zijlstra, linux-arch,
	Greg Kroah-Hartman

[-- Attachment #1: Type: TEXT/PLAIN, Size: 661 bytes --]

On Fri, 11 Feb 2011, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
> >  129 files changed, 4353 insertions(+), 5400 deletions(-)
> 
> Did I get the file count right - roughtly how many files are affected throughout all 
> architectures? The changes are massively intrusive and widely spread out so we 
> cannot do them in one go, right?

Alone the stuff which fiddles in irq_desc->status for whatever reasons
is >100 files in arch/. And that has do be done manually case by case.

The namespace cleanup affects about 300 files in arch, but that can be
done in one go right after -rc1 if I manage to get the regex straight :)

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-11 14:26             ` Thomas Gleixner
  2011-02-11 14:26               ` Thomas Gleixner
@ 2011-02-13 12:50               ` Sam Ravnborg
  2011-02-14 19:01                 ` Thomas Gleixner
  1 sibling, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2011-02-13 12:50 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, Linus Torvalds, LKML, Peter Zijlstra, linux-arch,
	Greg Kroah-Hartman

On Fri, Feb 11, 2011 at 03:26:11PM +0100, Thomas Gleixner wrote:
> On Fri, 11 Feb 2011, Ingo Molnar wrote:
> > * Thomas Gleixner <tglx@linutronix.de> wrote:
> > >  129 files changed, 4353 insertions(+), 5400 deletions(-)
> > 
> > Did I get the file count right - roughtly how many files are affected throughout all 
> > architectures? The changes are massively intrusive and widely spread out so we 
> > cannot do them in one go, right?
> 
> Alone the stuff which fiddles in irq_desc->status for whatever reasons
> is >100 files in arch/. And that has do be done manually case by case.

Some kind of arch TODO list wold be nice. Just to let the arch maintainers
know what you expect from them (on top of the deprecated warnings).

	Sam

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 00/75] genirq: Overhaul for 2.6.39
  2011-02-13 12:50               ` Sam Ravnborg
@ 2011-02-14 19:01                 ` Thomas Gleixner
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2011-02-14 19:01 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Ingo Molnar, Linus Torvalds, LKML, Peter Zijlstra, linux-arch,
	Greg Kroah-Hartman

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1192 bytes --]

On Sun, 13 Feb 2011, Sam Ravnborg wrote:

> On Fri, Feb 11, 2011 at 03:26:11PM +0100, Thomas Gleixner wrote:
> > On Fri, 11 Feb 2011, Ingo Molnar wrote:
> > > * Thomas Gleixner <tglx@linutronix.de> wrote:
> > > >  129 files changed, 4353 insertions(+), 5400 deletions(-)
> > > 
> > > Did I get the file count right - roughtly how many files are affected throughout all 
> > > architectures? The changes are massively intrusive and widely spread out so we 
> > > cannot do them in one go, right?
> > 
> > Alone the stuff which fiddles in irq_desc->status for whatever reasons
> > is >100 files in arch/. And that has do be done manually case by case.
> 
> Some kind of arch TODO list wold be nice. Just to let the arch maintainers
> know what you expect from them (on top of the deprecated warnings).

One thing which can be done right now is to use the proper existing
accessors irq_to_desc(), irq_set/clear_status_flags().

The remaining fixes depend on the yet to be merged queue. I will do
the namespace cleanup with cocinelle after rc1 as this is purely
mechanical. The other few things which will be left need manual
attendance, I will post either patches or a TODO list.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-02-14 19:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-10 23:35 [patch 00/75] genirq: Overhaul for 2.6.39 Thomas Gleixner
2011-02-10 23:53 ` Linus Torvalds
2011-02-11  0:00   ` Thomas Gleixner
2011-02-11  0:28     ` Linus Torvalds
2011-02-11  0:49       ` Thomas Gleixner
2011-02-11 13:05         ` Thomas Gleixner
2011-02-11 13:59           ` Ingo Molnar
2011-02-11 14:26             ` Thomas Gleixner
2011-02-11 14:26               ` Thomas Gleixner
2011-02-13 12:50               ` Sam Ravnborg
2011-02-14 19:01                 ` Thomas Gleixner
2011-02-11  4:03 ` Frank Rowand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox