Hi,
Recent e1000 updates introduced variable declarations after code. Fix
those up again.
Signed-off-by: Jens Axboe <[email protected]>
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index d0a5d16..ca68a04 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
e1000_pci_set_mwi(&adapter->hw);
if(netif_running(netdev)) {
+ struct e1000_rx_ring *ring;
+
e1000_configure_rx(adapter);
/* No need to loop, because 82542 supports only 1 queue */
- struct e1000_rx_ring *ring = &adapter->rx_ring[0];
+ ring = &adapter->rx_ring[0];
adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
}
}
@@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
rx_desc = E1000_RX_DESC(*rx_ring, i);
while(rx_desc->status & E1000_RXD_STAT_DD) {
- buffer_info = &rx_ring->buffer_info[i];
u8 status;
+ buffer_info = &rx_ring->buffer_info[i];
#ifdef CONFIG_E1000_NAPI
if(*work_done >= work_to_do)
break;
--
Jens Axboe
Hi,
Seems e1000 has even bigger problems than just C style badness in the
newest -git after the e1000 updates. diff of kernel boot messages from
e1000:
-Intel(R) PRO/1000 Network Driver - version 6.1.16-k2
+Intel(R) PRO/1000 Network Driver - version 6.3.9-k2
Copyright (c) 1999-2005 Intel Corporation.
ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 16 (level, low) -> IRQ 169
PCI: Setting latency timer of device 0000:02:00.0 to 64
+e1000: 0000:02:00.0: e1000_probe: (PCI Express:2.5Gb/s:Width x1)
00:0c:f1:ff:92:b8
e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
It sends/receives 5 packages, but the dhcp request never succeeds. If I
down the interface, it oopses (nvidia module is loaded, however it bombs
in the same way without it). If I revert just the e1000 updates, my
system work fine (typing this message from it).
Unable to handle kernel NULL pointer dereference at 0000000000000160 RIP:
<ffffffff80319d0b>{skb_drop_fraglist+20}
PGD 0
Oops: 0000 [1] SMP
CPU 1
Modules linked in: nvidia
Pid: 2806, comm: dhcpcd Tainted: P 2.6.16-rc1 #15
RIP: 0010:[<ffffffff80319d0b>] <ffffffff80319d0b>{skb_drop_fraglist+20}
RSP: 0018:ffff81003a39fce8 EFLAGS: 00010206
RAX: ffff81003c8dce80 RBX: ffff81003e28ec80 RCX: 0000000000000002
RDX: 0000000000000800 RSI: 000000003e32c012 RDI: 0000000000000160
RBP: ffff81003f33a1c0 R08: 000000000000000f R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffff81003f345000 R14: ffff81003f011000 R15: ffff81003a162610
FS: 00002aff325eab00(0000) GS:ffff81003f2ce440(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000160 CR3: 000000003a617000 CR4: 00000000000006e0
Process dhcpcd (pid: 2806, threadinfo ffff81003a39e000, task ffff81003f22eea0)
Stack: ffff81003e28ec80 ffffffff80319dc4 0000000000000000 ffff81003e28ec80
ffff81003f33a1c0 ffffffff80319de2 ffffc20000036000 ffffffff80293534
ffff81003a39fd56 ffff81003f011500
Call Trace: <ffffffff80319dc4>{skb_release_data+136}
<ffffffff80319de2>{kfree_skbmem+9} <ffffffff80293534>{e1000_clean_rx_ring+137}
<ffffffff80293696>{e1000_clean_all_rx_rings+32} <ffffffff80297c85>{e1000_down+279}
<ffffffff802983d5>{e1000_close+21} <ffffffff8031cf11>{dev_close+85}
<ffffffff8031cd1a>{dev_change_flags+97} <ffffffff80352f1e>{inet_ioctl+0}
<ffffffff803528d8>{devinet_ioctl+542} <ffffffff80314a86>{sock_ioctl+500}
<ffffffff80173f45>{do_ioctl+33} <ffffffff801741c9>{vfs_ioctl+570}
<ffffffff8017421e>{sys_ioctl+60} <ffffffff8010a81e>{system_call+126}
Code: 48 8b 1f 8b 87 ac 00 00 00 ff c8 75 05 0f ae e8 eb 0e f0 ff
RIP <ffffffff80319d0b>{skb_drop_fraglist+20} RSP <ffff81003a39fce8>
CR2: 0000000000000160
--
Jens Axboe
Jens Axboe ha scritto:
> Hi,
>
> Recent e1000 updates introduced variable declarations after code. Fix
> those up again.
>
> Signed-off-by: Jens Axboe <[email protected]>
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index d0a5d16..ca68a04 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
> e1000_pci_set_mwi(&adapter->hw);
>
> if(netif_running(netdev)) {
> + struct e1000_rx_ring *ring;
> +
> e1000_configure_rx(adapter);
> /* No need to loop, because 82542 supports only 1 queue */
> - struct e1000_rx_ring *ring = &adapter->rx_ring[0];
> + ring = &adapter->rx_ring[0];
> adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
> }
> }
> @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
> rx_desc = E1000_RX_DESC(*rx_ring, i);
>
> while(rx_desc->status & E1000_RXD_STAT_DD) {
> - buffer_info = &rx_ring->buffer_info[i];
> u8 status;
> + buffer_info = &rx_ring->buffer_info[i];
> #ifdef CONFIG_E1000_NAPI
> if(*work_done >= work_to_do)
> break;
>
Shouldn't variables declaration be on top of function and not on top of
a block (like if, while, for...)?
--
Patrizio Bassi
http://www.patriziobassi.it
On Wed, Jan 18 2006, Patrizio Bassi wrote:
> Jens Axboe ha scritto:
> > Hi,
> >
> > Recent e1000 updates introduced variable declarations after code. Fix
> > those up again.
> >
> > Signed-off-by: Jens Axboe <[email protected]>
> >
> > diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> > index d0a5d16..ca68a04 100644
> > --- a/drivers/net/e1000/e1000_main.c
> > +++ b/drivers/net/e1000/e1000_main.c
> > @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
> > e1000_pci_set_mwi(&adapter->hw);
> >
> > if(netif_running(netdev)) {
> > + struct e1000_rx_ring *ring;
> > +
> > e1000_configure_rx(adapter);
> > /* No need to loop, because 82542 supports only 1 queue */
> > - struct e1000_rx_ring *ring = &adapter->rx_ring[0];
> > + ring = &adapter->rx_ring[0];
> > adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
> > }
> > }
> > @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
> > rx_desc = E1000_RX_DESC(*rx_ring, i);
> >
> > while(rx_desc->status & E1000_RXD_STAT_DD) {
> > - buffer_info = &rx_ring->buffer_info[i];
> > u8 status;
> > + buffer_info = &rx_ring->buffer_info[i];
> > #ifdef CONFIG_E1000_NAPI
> > if(*work_done >= work_to_do)
> > break;
> >
>
> Shouldn't variables declaration be on top of function and not on top of
> a block (like if, while, for...)?
No, that's not necessary. But they should be before actual code inside
that block.
--
Jens Axboe
Patrizio Bassi wrote:
> Jens Axboe ha scritto:
>
>>Hi,
>>
>>Recent e1000 updates introduced variable declarations after code. Fix
>>those up again.
>>
>>Signed-off-by: Jens Axboe <[email protected]>
>>
>>diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
>>index d0a5d16..ca68a04 100644
>>--- a/drivers/net/e1000/e1000_main.c
>>+++ b/drivers/net/e1000/e1000_main.c
>>@@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
>> e1000_pci_set_mwi(&adapter->hw);
>>
>> if(netif_running(netdev)) {
>>+ struct e1000_rx_ring *ring;
>>+
>> e1000_configure_rx(adapter);
>> /* No need to loop, because 82542 supports only 1 queue */
>>- struct e1000_rx_ring *ring = &adapter->rx_ring[0];
>>+ ring = &adapter->rx_ring[0];
>> adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
>> }
>> }
>>@@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
>> rx_desc = E1000_RX_DESC(*rx_ring, i);
>>
>> while(rx_desc->status & E1000_RXD_STAT_DD) {
>>- buffer_info = &rx_ring->buffer_info[i];
>> u8 status;
>>+ buffer_info = &rx_ring->buffer_info[i];
>> #ifdef CONFIG_E1000_NAPI
>> if(*work_done >= work_to_do)
>> break;
>>
>
>
> Shouldn't variables declaration be on top of function and not on top of
> a block (like if, while, for...)?
>
Any block is OK, and they all have the same nice symmetry - variables
come into scope at the top and go out of scope at the bottom.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
Nick Piggin ha scritto:
> Patrizio Bassi wrote:
>> Jens Axboe ha scritto:
>>
>>> Hi,
>>>
>>> Recent e1000 updates introduced variable declarations after code. Fix
>>> those up again.
>>>
>>> Signed-off-by: Jens Axboe <[email protected]>
>>>
>>> diff --git a/drivers/net/e1000/e1000_main.c
>>> b/drivers/net/e1000/e1000_main.c
>>> index d0a5d16..ca68a04 100644
>>> --- a/drivers/net/e1000/e1000_main.c
>>> +++ b/drivers/net/e1000/e1000_main.c
>>> @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
>>> e1000_pci_set_mwi(&adapter->hw);
>>>
>>> if(netif_running(netdev)) {
>>> + struct e1000_rx_ring *ring;
>>> +
>>> e1000_configure_rx(adapter);
>>> /* No need to loop, because 82542 supports only 1 queue */
>>> - struct e1000_rx_ring *ring = &adapter->rx_ring[0];
>>> + ring = &adapter->rx_ring[0];
>>> adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
>>> }
>>> }
>>> @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
>>> rx_desc = E1000_RX_DESC(*rx_ring, i);
>>>
>>> while(rx_desc->status & E1000_RXD_STAT_DD) {
>>> - buffer_info = &rx_ring->buffer_info[i];
>>> u8 status;
>>> + buffer_info = &rx_ring->buffer_info[i];
>>> #ifdef CONFIG_E1000_NAPI
>>> if(*work_done >= work_to_do)
>>> break;
>>>
>>
>>
>> Shouldn't variables declaration be on top of function and not on top of
>> a block (like if, while, for...)?
>>
>
> Any block is OK, and they all have the same nice symmetry - variables
> come into scope at the top and go out of scope at the bottom.
>
ok, i'm still linked to the 70' C style (not confortable) :)
old compilers failed with such syntax.
On 1/18/06, Jens Axboe <[email protected]> wrote:
> Hi,
>
> Seems e1000 has even bigger problems than just C style badness in the
> newest -git after the e1000 updates. diff of kernel boot messages from
> e1000:
>
> -Intel(R) PRO/1000 Network Driver - version 6.1.16-k2
> +Intel(R) PRO/1000 Network Driver - version 6.3.9-k2
> Copyright (c) 1999-2005 Intel Corporation.
> ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 16 (level, low) -> IRQ 169
> PCI: Setting latency timer of device 0000:02:00.0 to 64
> +e1000: 0000:02:00.0: e1000_probe: (PCI Express:2.5Gb/s:Width x1)
> 00:0c:f1:ff:92:b8
> e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
>
> It sends/receives 5 packages, but the dhcp request never succeeds. If I
> down the interface, it oopses (nvidia module is loaded, however it bombs
> in the same way without it). If I revert just the e1000 updates, my
> system work fine (typing this message from it).
e1000 is failing the same way for me too. Chip is on the motherboard:
Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)
Jan 18 10:08:26 jonsmirl kernel: ADDRCONF(NETDEV_UP): eth0: link is not ready
Jan 18 10:08:26 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
Link is Up 100 Mbps Half Duplex
Jan 18 10:08:26 jonsmirl kernel: ADDRCONF(NETDEV_CHANGE): eth0: link
becomes ready
Jan 18 10:08:26 jonsmirl dhclient: DHCPREQUEST on eth0 to
255.255.255.255 port 67
Jan 18 10:08:31 jonsmirl dhclient: DHCPREQUEST on eth0 to
255.255.255.255 port 67
Jan 18 10:08:38 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 8
Jan 18 10:08:46 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 10
Jan 18 10:08:56 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 11
Jan 18 10:09:07 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 20
Jan 18 10:09:27 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 9
Jan 18 10:09:36 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 3
Jan 18 10:09:39 jonsmirl dhclient: No DHCPOFFERS received.
I am using an old switch so I normally get a half duplex connection:
Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
Link is Up 100 Mbps Half Duplex
I don't normally get the NETDEV up and changed notifications.
--
Jon Smirl
[email protected]
Jon Smirl wrote:
> e1000 is failing the same way for me too. Chip is on the motherboard:
> Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)
[...]
> I am using an old switch so I normally get a half duplex connection:
> Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
> Link is Up 100 Mbps Half Duplex
> I don't normally get the NETDEV up and changed notifications.
e1000 is failing for everybody :( Looks like somewhere in this 40-patch
behemoth, an "e1000 doesn't work anymore" change slipped in.
We'll get it fixed ASAP, even if it means reverting all those patches.
Jeff
On Wed, Jan 18 2006, Jeff Garzik wrote:
> Jon Smirl wrote:
> >e1000 is failing the same way for me too. Chip is on the motherboard:
> >Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)
> [...]
> >I am using an old switch so I normally get a half duplex connection:
> >Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
> >Link is Up 100 Mbps Half Duplex
> >I don't normally get the NETDEV up and changed notifications.
>
> e1000 is failing for everybody :( Looks like somewhere in this 40-patch
> behemoth, an "e1000 doesn't work anymore" change slipped in.
>
> We'll get it fixed ASAP, even if it means reverting all those patches.
Thanks, it is rather critical. At least it didn't get included in -rc1,
that would have been a disaster :-)
--
Jens Axboe
On 1/18/06, Jens Axboe <[email protected]> wrote:
> On Wed, Jan 18 2006, Jeff Garzik wrote:
> > We'll get it fixed ASAP, even if it means reverting all those patches.
>
> Thanks, it is rather critical. At least it didn't get included in -rc1,
> that would have been a disaster :-)
just FYI, I have a patch for the e1000 breakage which will be out as
soon as I can generate it.
Jesse
On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> On 1/18/06, Jens Axboe <[email protected]> wrote:
> > On Wed, Jan 18 2006, Jeff Garzik wrote:
> > > We'll get it fixed ASAP, even if it means reverting all those patches.
> >
> > Thanks, it is rather critical. At least it didn't get included in -rc1,
> > that would have been a disaster :-)
>
> just FYI, I have a patch for the e1000 breakage which will be out as
> soon as I can generate it.
Newest -git works for me. Well sort of, I get a lot of these:
e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
Tx Queue <0>
TDH <72>
TDT <e5>
next_to_use <e5>
next_to_clean <6f>
buffer_info[next_to_clean]
time_stamp <10000160a>
next_to_watch <72>
jiffies <100001e09>
next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
Tx Queue <0>
TDH <72>
TDT <e5>
next_to_use <e5>
next_to_clean <6f>
buffer_info[next_to_clean]
time_stamp <10000160a>
next_to_watch <72>
jiffies <1000025da>
next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
Tx Queue <0>
TDH <72>
TDT <e5>
next_to_use <e5>
next_to_clean <6f>
buffer_info[next_to_clean]
time_stamp <10000160a>
next_to_watch <72>
jiffies <10000357b>
next_to_watch.status <0>
NETDEV WATCHDOG: eth0: transmit timed out
e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
--
Jens Axboe
On 1/19/06, Jens Axboe <[email protected]> wrote:
> On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> > just FYI, I have a patch for the e1000 breakage which will be out as
> > soon as I can generate it.
>
> Newest -git works for me. Well sort of, I get a lot of these:
>
> e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> Tx Queue <0>
> TDH <72>
> TDT <e5>
> next_to_use <e5>
> next_to_clean <6f>
> buffer_info[next_to_clean]
> time_stamp <10000160a>
> next_to_watch <72>
> jiffies <100001e09>
> next_to_watch.status <0>
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> Tx Queue <0>
> TDH <72>
> TDT <e5>
> next_to_use <e5>
> next_to_clean <6f>
> buffer_info[next_to_clean]
> time_stamp <10000160a>
> next_to_watch <72>
> jiffies <1000025da>
> next_to_watch.status <0>
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> Tx Queue <0>
> TDH <72>
> TDT <e5>
> next_to_use <e5>
> next_to_clean <6f>
> buffer_info[next_to_clean]
> time_stamp <10000160a>
> next_to_watch <72>
> jiffies <10000357b>
> next_to_watch.status <0>
> NETDEV WATCHDOG: eth0: transmit timed out
> e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
where it didn't happen with the previous driver? I guess thats a good
thing, kinda as we made the problem more frequent, hopefully we can
help fix it?
you don't happen to have TSO enabled do you?
please reply over at netdev at vger.kernel.org with the standard set
of information, lspci, dmesg, etc etc.
Thanks,
Jesse
On Fri, Jan 20 2006, Jesse Brandeburg wrote:
> On 1/19/06, Jens Axboe <[email protected]> wrote:
> > On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> > > just FYI, I have a patch for the e1000 breakage which will be out as
> > > soon as I can generate it.
> >
> > Newest -git works for me. Well sort of, I get a lot of these:
> >
> > e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> > Tx Queue <0>
> > TDH <72>
> > TDT <e5>
> > next_to_use <e5>
> > next_to_clean <6f>
> > buffer_info[next_to_clean]
> > time_stamp <10000160a>
> > next_to_watch <72>
> > jiffies <100001e09>
> > next_to_watch.status <0>
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> > Tx Queue <0>
> > TDH <72>
> > TDT <e5>
> > next_to_use <e5>
> > next_to_clean <6f>
> > buffer_info[next_to_clean]
> > time_stamp <10000160a>
> > next_to_watch <72>
> > jiffies <1000025da>
> > next_to_watch.status <0>
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> > Tx Queue <0>
> > TDH <72>
> > TDT <e5>
> > next_to_use <e5>
> > next_to_clean <6f>
> > buffer_info[next_to_clean]
> > time_stamp <10000160a>
> > next_to_watch <72>
> > jiffies <10000357b>
> > next_to_watch.status <0>
> > NETDEV WATCHDOG: eth0: transmit timed out
> > e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
>
> where it didn't happen with the previous driver? I guess thats a good
> thing, kinda as we made the problem more frequent, hopefully we can
> help fix it?
Sorry I thought I had mentioned, it happens with the previous driver as
well. And it's pretty annoying, I get hickups due to these network
glitches.
> you don't happen to have TSO enabled do you?
Nope.
> please reply over at netdev at vger.kernel.org with the standard set
> of information, lspci, dmesg, etc etc.
Sure.
--
Jens Axboe