[PATCH] Do not include linux/irq.h from linux/netpoll.h

[PATCH] Do not include linux/irq.h from linux/netpoll.h

[Ian Campbell]

Hi,

(I hope netdev is the right place for netpoll stuff)

It seems that the changeset at
http://www.kernel.org/pub/linux/kernel/v2.6/testing/cset/cset-jgarzik@redhat.com|ChangeSet|20040302073919|27676.txt breaks the current BK tree build for ARM.

The culprit would appear to be the addition of a 
	#include <linux/netpoll.h>
to net/core/dev.c which in turn pulls in <linux/irq.h> which (as Russell
King notes in a comment therein) should not be included from generic
code.

>From what I can tell from the netpoll code used to call the drivers irq
handler to simulate a poll but now it uses the poll_controller function,
therefore I don't think the linux/irq.h needs to be included any longer.
The patch below removes the include.

I successfully built an ARM kernel with NETCONSOLE and NETPOLL enabled,
although I was not able to test it since my network driver has no poll
method.

Cheers,
Ian.

Index: linux-2.6-bkpxa/include/linux/netpoll.h
===================================================================
--- linux-2.6-bkpxa.orig/include/linux/netpoll.h        2004-03-15 15:03:30.000000000 +0000
+++ linux-2.6-bkpxa/include/linux/netpoll.h     2004-03-15 16:24:25.000000000 +0000
@@ -9,7 +9,6 @@
  
 #include <linux/netdevice.h>
 #include <linux/interrupt.h>
-#include <linux/irq.h>
 #include <linux/list.h>
  
 struct netpoll;

-- 
Ian Campbell, Senior Design Engineer
                                        Web: http://www.arcom.com
Arcom, Clifton Road, 			Direct: +44 (0)1223 403 465
Cambridge CB1 7EA, United Kingdom	Phone:  +44 (0)1223 411 200

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Russell King]

On Mon, Mar 15, 2004 at 04:52:50PM +0000, Ian Campbell wrote:
> The culprit would appear to be the addition of a 
> 	#include <linux/netpoll.h>
> to net/core/dev.c which in turn pulls in <linux/irq.h> which (as Russell
> King notes in a comment therein) should not be included from generic
> code.

Linus - I haven't tested this patch myself yet, but I do think something
needs to happen with linux/irq.h.  It seems a comment in the file isn't
sufficient.

The file itself is misplaced and misleading sitting in the include/linux
subdirectory, which causes problems when people decide to include it into
architecture independent files, in the belief that it's a generic include
file.

I believe that linux/irq.h should at least become asm-generic/irq.h to
stop this happening.

What are your thoughts on this?

(Unfortunately, bkbits browsing using lynx or links seems to be rather
broken at the moment, otherwise I'd have checked whether you've already
integrated this patch.)

Index: linux-2.6-bkpxa/include/linux/netpoll.h
===================================================================
--- linux-2.6-bkpxa.orig/include/linux/netpoll.h        2004-03-15 15:03:30.000000000 +0000
+++ linux-2.6-bkpxa/include/linux/netpoll.h     2004-03-15 16:24:25.000000000 +0000
@@ -9,7 +9,6 @@
  
 #include <linux/netdevice.h>
 #include <linux/interrupt.h>
-#include <linux/irq.h>
 #include <linux/list.h>
  
 struct netpoll;

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Russell King]

On Tue, Mar 16, 2004 at 12:11:41AM +0000, Russell King wrote:
> On Mon, Mar 15, 2004 at 04:52:50PM +0000, Ian Campbell wrote:
> > The culprit would appear to be the addition of a 
> > 	#include <linux/netpoll.h>
> > to net/core/dev.c which in turn pulls in <linux/irq.h> which (as Russell
> > King notes in a comment therein) should not be included from generic
> > code.
> 
> Linus - I haven't tested this patch myself yet, but I do think something
> needs to happen with linux/irq.h.  It seems a comment in the file isn't
> sufficient.
> 
> The file itself is misplaced and misleading sitting in the include/linux
> subdirectory, which causes problems when people decide to include it into
> architecture independent files, in the belief that it's a generic include
> file.
> 
> I believe that linux/irq.h should at least become asm-generic/irq.h to
> stop this happening.
> 
> What are your thoughts on this?

So how do we solve this problem.  Should I just merge this change and
ask you to pull it?  I think that's rather impolite though.

Or should I send a BK cset which removes include/linux/irq.h entirely,
thereby fixing _my_ problem (though it'll break everyone elses build.) 8)

> Index: linux-2.6-bkpxa/include/linux/netpoll.h
> ===================================================================
> --- linux-2.6-bkpxa.orig/include/linux/netpoll.h        2004-03-15 15:03:30.000000000 +0000
> +++ linux-2.6-bkpxa/include/linux/netpoll.h     2004-03-15 16:24:25.000000000 +0000
> @@ -9,7 +9,6 @@
>   
>  #include <linux/netdevice.h>
>  #include <linux/interrupt.h>
> -#include <linux/irq.h>
>  #include <linux/list.h>
>   
>  struct netpoll;
> 
> -- 
> Russell King
>  Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
>                  2.6 Serial core
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Christoph Hellwig]

On Tue, Mar 16, 2004 at 07:22:47PM +0000, Russell King wrote:
> So how do we solve this problem.  Should I just merge this change and
> ask you to pull it?  I think that's rather impolite though.
> 
> Or should I send a BK cset which removes include/linux/irq.h entirely,
> thereby fixing _my_ problem (though it'll break everyone elses build.) 8)

What about moving it to asm-generic now?  linux/irq.h never was a public
API so the stable API in 2.6 thing doesn't count.  And it's fix the confusing
for real.

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Linus Torvalds]

On Tue, 16 Mar 2004, Russell King wrote:
> > 
> > What are your thoughts on this?
> 
> So how do we solve this problem.  Should I just merge this change and
> ask you to pull it?  I think that's rather impolite though.

I didn't apply the patch because you said it was untested ;)

I'll happily remove that irq.h include if it really doesn't do anything 
but break things. I'd feel happier about it if somebody said it has been 
tested, though ;)

		Linus

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Russell King]

On Tue, Mar 16, 2004 at 11:34:56AM -0800, Linus Torvalds wrote:
> On Tue, 16 Mar 2004, Russell King wrote:
> > > 
> > > What are your thoughts on this?
> > 
> > So how do we solve this problem.  Should I just merge this change and
> > ask you to pull it?  I think that's rather impolite though.
> 
> I didn't apply the patch because you said it was untested ;)

Ok, but bear in mind that although I can test that removing linux/irq.h
from netpoll.h fixes my problem, it really needs an x86 person to also
test it, just in case there's some dependency there that may not show
up for me.

> I'll happily remove that irq.h include if it really doesn't do anything 
> but break things. I'd feel happier about it if somebody said it has been 
> tested, though ;)

Andi Kleen, hch and jgarzik are presently discussing the issue, and I
think they're convincing themselves that linux/irq.h is disgusting
mess.

As far as me doing anything with linux/irq.h, I think that's out of my
control because ARM doesn't use it - an x86 person needs to look into
fixing it properly.

So all I can do is moan each time this problem comes up until someone
gets pissed off enough to fix it properly.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Russell King]

On Tue, Mar 16, 2004 at 11:34:56AM -0800, Linus Torvalds wrote:
> On Tue, 16 Mar 2004, Russell King wrote:
> > > 
> > > What are your thoughts on this?
> > 
> > So how do we solve this problem.  Should I just merge this change and
> > ask you to pull it?  I think that's rather impolite though.
> 
> I didn't apply the patch because you said it was untested ;)

Ok, I've now tested the removal of linux/irq.h from netpoll.h and it
built fine for me on ARM.

However, whether there's a reason for it to be there on x86 or not
still remains to be proven.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Do not include linux/irq.h from linux/netpoll.h

[Adrian Bunk]

On Tue, Mar 16, 2004 at 11:34:56AM -0800, Linus Torvalds wrote:
> 
> 
> On Tue, 16 Mar 2004, Russell King wrote:
> > > 
> > > What are your thoughts on this?
> > 
> > So how do we solve this problem.  Should I just merge this change and
> > ask you to pull it?  I think that's rather impolite though.
> 
> I didn't apply the patch because you said it was untested ;)
> 
> I'll happily remove that irq.h include if it really doesn't do anything 
> but break things. I'd feel happier about it if somebody said it has been 
> tested, though ;)

I removed the irq.h from netpoll.h in a 2.6.5-rc1-mm1 that was compiled
with a i386 .config that compiles as much as possible statically into
the kernel.

There were no compilation problems.

> 		Linus

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed