Re: 2.6.17-mm6

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* Re: 2.6.17-mm6
       [not found] ` <200607042153.31848.rjw@sisk.pl>
@ 2006-07-04 20:01   ` Arjan van de Ven
  2006-07-05 10:27     ` 2.6.17-mm6 Stefan Richter
  0 siblings, 1 reply; 6+ messages in thread
From: Arjan van de Ven @ 2006-07-04 20:01 UTC (permalink / raw)
  To: netdev, Rafael J. Wysocki; +Cc: Andrew Morton, linux-kernel

this is one for the networking people, and thus netdev


On Tue, 2006-07-04 at 21:53 +0200, Rafael J. Wysocki wrote:
> On Monday 03 July 2006 12:03, Andrew Morton wrote:
> > 
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17/2.6.17-mm6/
> > 
> > 
> > - A major update to the e1000 driver.
> > 
> > - 1394 updates
> 
> Just found this in dmesg:
> 
> =================================
> [ INFO: inconsistent lock state ]
> ---------------------------------
> inconsistent {in-hardirq-W} -> {hardirq-on-W} usage.
> nscd/4929 [HC0[0]:SC0[1]:HE1:SE0] takes:
>  (&skb_queue_lock_key){++..}, at: [<ffffffff8044fe40>] udp_ioctl+0x50/0xa0
> {in-hardirq-W} state was registered at:
>   [<ffffffff8024b4fa>] lock_acquire+0x8a/0xc0
>   [<ffffffff80476e3f>] _spin_lock_irqsave+0x3f/0x60
>   [<ffffffff80408c25>] skb_queue_tail+0x25/0x60

ok so skb_queue_lock is used in a hardirq context

>   [<ffffffff881c9517>] queue_packet_complete+0x27/0x40 [ieee1394]
>   [<ffffffff881c9d6b>] hpsb_packet_sent+0xab/0x100 [ieee1394]
>   [<ffffffff8822a4b5>] dma_trm_reset+0x115/0x140 [ohci1394]
>   [<ffffffff8822c512>] ohci_devctl+0x1c2/0x540 [ohci1394]
>   [<ffffffff881c9673>] hpsb_bus_reset+0x43/0xb0 [ieee1394]
>   [<ffffffff8822d7f6>] ohci_irq_handler+0x416/0x830 [ohci1394]
>   [<ffffffff802631ab>] handle_IRQ_event+0x2b/0x70
>   [<ffffffff80264dd4>] handle_level_irq+0xc4/0x130
>   [<ffffffff8020c762>] do_IRQ+0x112/0x130
>   [<ffffffff80209d90>] common_interrupt+0x64/0x65
> irq event stamp: 4280
> hardirqs last  enabled at (4279): [<ffffffff8047606a>] trace_hardirqs_on_thunk+0x35/0x37
> hardirqs last disabled at (4278): [<ffffffff804760a1>] trace_hardirqs_off_thunk+0x35/0x67
> softirqs last  enabled at (4258): [<ffffffff804065b5>] release_sock+0xd5/0xe0
> softirqs last disabled at (4280): [<ffffffff804764d1>] _spin_lock_bh+0x11/0x50
> 
> other info that might help us debug this:
> no locks held by nscd/4929.
> 
> stack backtrace:
> 
> Call Trace:
>  [<ffffffff8020ab9f>] show_trace+0x9f/0x240
>  [<ffffffff8020af75>] dump_stack+0x15/0x20
>  [<ffffffff80249e52>] print_usage_bug+0x272/0x290
>  [<ffffffff8024a0d7>] mark_lock+0x267/0x5f0
>  [<ffffffff8024a9a6>] __lock_acquire+0x546/0xd10
>  [<ffffffff8024b4fb>] lock_acquire+0x8b/0xc0
>  [<ffffffff804764f4>] _spin_lock_bh+0x34/0x50
>  [<ffffffff8044fe40>] udp_ioctl+0x50/0xa0

yet udp_ioctl takes it only for _bh

>  [<ffffffff80457359>] inet_ioctl+0x69/0x70
>  [<ffffffff804033ac>] sock_ioctl+0x22c/0x270
>  [<ffffffff802a32b1>] do_ioctl+0x31/0xa0
>  [<ffffffff802a35db>] vfs_ioctl+0x2bb/0x2e0
>  [<ffffffff802a366a>] sys_ioctl+0x6a/0xa0
>  [<ffffffff8020985a>] system_call+0x7e/0x83
>  [<00002b2d76ab98a9>]


is this a real scenario, or is this a case of "firewire is special and
needs it's own rules"?



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

* Re: 2.6.17-mm6
  2006-07-04 20:01   ` 2.6.17-mm6 Arjan van de Ven
@ 2006-07-05 10:27     ` Stefan Richter
  2006-07-05 10:36       ` 2.6.17-mm6 Stefan Richter
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Richter @ 2006-07-05 10:27 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: netdev, Rafael J. Wysocki, Andrew Morton, linux-kernel,
	Ingo Molnar

On 7/4/2006 10:01 PM, Arjan van de Ven wrote:
> this is one for the networking people, and thus netdev

It's actually ieee1394 using net infrastructure for purposes which ar
unrelated to networking.

Furthermore...

> On Tue, 2006-07-04 at 21:53 +0200, Rafael J. Wysocki wrote:
>> On Monday 03 July 2006 12:03, Andrew Morton wrote:
>> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17/2.6.17-mm6/
>> > 
>> > - A major update to the e1000 driver.
>> > - 1394 updates

...I believe it is unrelated to the 1394 updates new to -mm6.

>> Just found this in dmesg:
>> 
>> =================================
>> [ INFO: inconsistent lock state ]
>> ---------------------------------
>> inconsistent {in-hardirq-W} -> {hardirq-on-W} usage.
>> nscd/4929 [HC0[0]:SC0[1]:HE1:SE0] takes:
>>  (&skb_queue_lock_key){++..}, at: [<ffffffff8044fe40>] udp_ioctl+0x50/0xa0
>> {in-hardirq-W} state was registered at:
>>   [<ffffffff8024b4fa>] lock_acquire+0x8a/0xc0
>>   [<ffffffff80476e3f>] _spin_lock_irqsave+0x3f/0x60
>>   [<ffffffff80408c25>] skb_queue_tail+0x25/0x60
> 
> ok so skb_queue_lock is used in a hardirq context
> 
>>   [<ffffffff881c9517>] queue_packet_complete+0x27/0x40 [ieee1394]
>>   [<ffffffff881c9d6b>] hpsb_packet_sent+0xab/0x100 [ieee1394]
>>   [<ffffffff8822a4b5>] dma_trm_reset+0x115/0x140 [ohci1394]
>>   [<ffffffff8822c512>] ohci_devctl+0x1c2/0x540 [ohci1394]
>>   [<ffffffff881c9673>] hpsb_bus_reset+0x43/0xb0 [ieee1394]
>>   [<ffffffff8822d7f6>] ohci_irq_handler+0x416/0x830 [ohci1394]
>>   [<ffffffff802631ab>] handle_IRQ_event+0x2b/0x70
>>   [<ffffffff80264dd4>] handle_level_irq+0xc4/0x130
>>   [<ffffffff8020c762>] do_IRQ+0x112/0x130
>>   [<ffffffff80209d90>] common_interrupt+0x64/0x65
>> irq event stamp: 4280
>> hardirqs last  enabled at (4279): [<ffffffff8047606a>] trace_hardirqs_on_thunk+0x35/0x37
>> hardirqs last disabled at (4278): [<ffffffff804760a1>] trace_hardirqs_off_thunk+0x35/0x67
>> softirqs last  enabled at (4258): [<ffffffff804065b5>] release_sock+0xd5/0xe0
>> softirqs last disabled at (4280): [<ffffffff804764d1>] _spin_lock_bh+0x11/0x50
>> 
>> other info that might help us debug this:
>> no locks held by nscd/4929.
>> 
>> stack backtrace:
>> 
>> Call Trace:
>>  [<ffffffff8020ab9f>] show_trace+0x9f/0x240
>>  [<ffffffff8020af75>] dump_stack+0x15/0x20
>>  [<ffffffff80249e52>] print_usage_bug+0x272/0x290
>>  [<ffffffff8024a0d7>] mark_lock+0x267/0x5f0
>>  [<ffffffff8024a9a6>] __lock_acquire+0x546/0xd10
>>  [<ffffffff8024b4fb>] lock_acquire+0x8b/0xc0
>>  [<ffffffff804764f4>] _spin_lock_bh+0x34/0x50
>>  [<ffffffff8044fe40>] udp_ioctl+0x50/0xa0
> 
> yet udp_ioctl takes it only for _bh
> 
>>  [<ffffffff80457359>] inet_ioctl+0x69/0x70
>>  [<ffffffff804033ac>] sock_ioctl+0x22c/0x270
>>  [<ffffffff802a32b1>] do_ioctl+0x31/0xa0
>>  [<ffffffff802a35db>] vfs_ioctl+0x2bb/0x2e0
>>  [<ffffffff802a366a>] sys_ioctl+0x6a/0xa0
>>  [<ffffffff8020985a>] system_call+0x7e/0x83
>>  [<00002b2d76ab98a9>]
> 
> is this a real scenario, or is this a case of "firewire is special and
> needs it's own rules"?

Well, firewire is special, but that should already be addressed by this
patch: "lockdep: annotate ieee1394 skb-queue-head locking"
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff_plain;h=d378834840907326ac9d448056d957d13cc3718f

Why is there still a lockdep warning?

(Ieee1394 core's usage of the skb_* API is entirely unrelated to
networking; even if eth1394 was used.)
-- 
Stefan Richter
-=====-=-==- -=== --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.17-mm6
  2006-07-05 10:27     ` 2.6.17-mm6 Stefan Richter
@ 2006-07-05 10:36       ` Stefan Richter
  2006-07-05 11:13         ` 2.6.17-mm6 Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Richter @ 2006-07-05 10:36 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: netdev, Rafael J. Wysocki, Andrew Morton, linux-kernel,
	Ingo Molnar

I wrote:
> (Ieee1394 core's usage of the skb_* API is entirely unrelated to
> networking; even if eth1394 was used.)

PS:
I wonder if it wouldn't be better to migrate ieee1394 core away from
skb_*. I didn't look thoroughly at it yet but the benefit of using this
API appears quite low to me.

We use it to keep track of IEEE 1394 transactions [ = outgoing request
&& (incoming response || expiry)], with completion of transactions often
in-order due to mostly single-threaded usage, but sometimes out-of-order
(may happen regardless of multithreaded or single-threaded usage).
-- 
Stefan Richter
-=====-=-==- -=== --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.17-mm6
  2006-07-05 10:36       ` 2.6.17-mm6 Stefan Richter
@ 2006-07-05 11:13         ` Ingo Molnar
  0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2006-07-05 11:13 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Arjan van de Ven, netdev, Rafael J. Wysocki, Andrew Morton,
	linux-kernel


* Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> I wrote:
> > (Ieee1394 core's usage of the skb_* API is entirely unrelated to
> > networking; even if eth1394 was used.)
> 
> PS:
> I wonder if it wouldn't be better to migrate ieee1394 core away from 
> skb_*. I didn't look thoroughly at it yet but the benefit of using 
> this API appears quite low to me.

yeah, it seems to be the wrong abstraction to use. It's also more 
expensive than necessary: e.g. skb-heads have a qlen field that is 
maintained in every list op - but the ieee1394 code does not make use of 
the queue-length information. Using list.h plus a spinlock should do the 
trick?

	Ingo

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

* [-mm patch] drivers/net/e1000/: possible cleanups
       [not found] <20060703030355.420c7155.akpm@osdl.org>
       [not found] ` <200607042153.31848.rjw@sisk.pl>
@ 2006-07-06 20:37 ` Adrian Bunk
  2006-07-07 15:24 ` 2.6.17-mm6 Reuben Farrelly
  2 siblings, 0 replies; 6+ messages in thread
From: Adrian Bunk @ 2006-07-06 20:37 UTC (permalink / raw)
  To: Andrew Morton, cramerj, john.ronciak, jesse.brandeburg,
	jeffrey.t.kirsher, auke-jan.h.kok
  Cc: linux-kernel, jgarzik, netdev

On Mon, Jul 03, 2006 at 03:03:55AM -0700, Andrew Morton wrote:
>...
> Changes since 2.6.17-mm5:
>...
>  git-e1000.patch
>...
>  git trees.
>...

This patch contains the following possible cleanups:
- make needlessly global functions static
- #if 0 the following unused global functions:
  - e1000_hw.c: e1000_mc_addr_list_update()
  - e1000_hw.c: e1000_read_reg_io()
  - e1000_hw.c: e1000_enable_pciex_master()
  - e1000_hw.c: e1000_ife_disable_dynamic_power_down()
  - e1000_hw.c: e1000_ife_enable_dynamic_power_down()
  - e1000_hw.c: e1000_write_ich8_word()
  - e1000_hw.c: e1000_duplex_reversal()
  - e1000_main.c: e1000_io_read()

Please review which of these changes do make sense and which do 
conflict with pending patches.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 drivers/net/e1000/e1000_hw.c   |   89 ++++++++++++++++++++++++---------
 drivers/net/e1000/e1000_hw.h   |   32 -----------
 drivers/net/e1000/e1000_main.c |    2 
 3 files changed, 67 insertions(+), 56 deletions(-)

--- linux-2.6.17-mm6-full/drivers/net/e1000/e1000_hw.h.old	2006-07-06 21:17:20.000000000 +0200
+++ linux-2.6.17-mm6-full/drivers/net/e1000/e1000_hw.h	2006-07-06 21:57:18.000000000 +0200
@@ -323,13 +323,8 @@
 int32_t e1000_phy_hw_reset(struct e1000_hw *hw);
 int32_t e1000_phy_reset(struct e1000_hw *hw);
 void e1000_phy_powerdown_workaround(struct e1000_hw *hw);
-int32_t e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw);
-int32_t e1000_init_lcd_from_nvm_config_region(struct e1000_hw *hw, uint32_t cnf_base_addr, uint32_t cnf_size);
-int32_t e1000_init_lcd_from_nvm(struct e1000_hw *hw);
 int32_t e1000_phy_get_info(struct e1000_hw *hw, struct e1000_phy_info *phy_info);
 int32_t e1000_validate_mdi_setting(struct e1000_hw *hw);
-int32_t e1000_read_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, uint16_t *data);
-int32_t e1000_write_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, uint16_t data);
 
 /* EEPROM Functions */
 int32_t e1000_init_eeprom_params(struct e1000_hw *hw);
@@ -400,13 +395,8 @@
 int32_t e1000_write_eeprom(struct e1000_hw *hw, uint16_t reg, uint16_t words, uint16_t *data);
 int32_t e1000_read_part_num(struct e1000_hw *hw, uint32_t * part_num);
 int32_t e1000_read_mac_addr(struct e1000_hw * hw);
-int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask);
-void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask);
-void e1000_release_software_flag(struct e1000_hw *hw);
-int32_t e1000_get_software_flag(struct e1000_hw *hw);
 
 /* Filters (multicast, vlan, receive) */
-void e1000_mc_addr_list_update(struct e1000_hw *hw, uint8_t * mc_addr_list, uint32_t mc_addr_count, uint32_t pad, uint32_t rar_used_count);
 uint32_t e1000_hash_mc_addr(struct e1000_hw *hw, uint8_t * mc_addr);
 void e1000_mta_set(struct e1000_hw *hw, uint32_t hash_value);
 void e1000_rar_set(struct e1000_hw *hw, uint8_t * mc_addr, uint32_t rar_index);
@@ -431,31 +421,9 @@
 void e1000_read_pci_cfg(struct e1000_hw *hw, uint32_t reg, uint16_t * value);
 void e1000_write_pci_cfg(struct e1000_hw *hw, uint32_t reg, uint16_t * value);
 /* Port I/O is only supported on 82544 and newer */
-uint32_t e1000_io_read(struct e1000_hw *hw, unsigned long port);
-uint32_t e1000_read_reg_io(struct e1000_hw *hw, uint32_t offset);
 void e1000_io_write(struct e1000_hw *hw, unsigned long port, uint32_t value);
-void e1000_enable_pciex_master(struct e1000_hw *hw);
 int32_t e1000_disable_pciex_master(struct e1000_hw *hw);
-int32_t e1000_get_software_semaphore(struct e1000_hw *hw);
-void e1000_release_software_semaphore(struct e1000_hw *hw);
 int32_t e1000_check_phy_reset_block(struct e1000_hw *hw);
-int32_t e1000_set_pci_ex_no_snoop(struct e1000_hw *hw, uint32_t no_snoop);
-
-int32_t e1000_read_ich8_byte(struct e1000_hw *hw, uint32_t index,
-                             uint8_t *data);
-int32_t e1000_verify_write_ich8_byte(struct e1000_hw *hw, uint32_t index,
-                                     uint8_t byte);
-int32_t e1000_write_ich8_byte(struct e1000_hw *hw, uint32_t index,
-                              uint8_t byte);
-int32_t e1000_read_ich8_word(struct e1000_hw *hw, uint32_t index,
-                             uint16_t *data);
-int32_t e1000_read_ich8_data(struct e1000_hw *hw, uint32_t index,
-                             uint32_t size, uint16_t *data);
-int32_t e1000_read_eeprom_ich8(struct e1000_hw *hw, uint16_t offset,
-                               uint16_t words, uint16_t *data);
-int32_t e1000_write_eeprom_ich8(struct e1000_hw *hw, uint16_t offset,
-                                uint16_t words, uint16_t *data);
-int32_t e1000_erase_ich8_4k_segment(struct e1000_hw *hw, uint32_t segment);
 
 
 #define E1000_READ_REG_IO(a, reg) \
--- linux-2.6.17-mm6-full/drivers/net/e1000/e1000_hw.c.old	2006-07-06 21:16:17.000000000 +0200
+++ linux-2.6.17-mm6-full/drivers/net/e1000/e1000_hw.c	2006-07-06 21:39:03.000000000 +0200
@@ -105,6 +105,33 @@
                                                uint16_t duplex);
 static int32_t e1000_configure_kmrn_for_1000(struct e1000_hw *hw);
 
+static int32_t e1000_erase_ich8_4k_segment(struct e1000_hw *hw,
+					   uint32_t segment);
+static int32_t e1000_get_software_flag(struct e1000_hw *hw);
+static int32_t e1000_get_software_semaphore(struct e1000_hw *hw);
+static int32_t e1000_init_lcd_from_nvm(struct e1000_hw *hw);
+static int32_t e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw);
+static int32_t e1000_read_eeprom_ich8(struct e1000_hw *hw, uint16_t offset,
+				      uint16_t words, uint16_t *data);
+static int32_t e1000_read_ich8_byte(struct e1000_hw *hw, uint32_t index,
+				    uint8_t* data);
+static int32_t e1000_read_ich8_word(struct e1000_hw *hw, uint32_t index,
+				    uint16_t *data);
+static int32_t e1000_read_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr,
+				   uint16_t *data);
+static void e1000_release_software_flag(struct e1000_hw *hw);
+static void e1000_release_software_semaphore(struct e1000_hw *hw);
+static int32_t e1000_set_pci_ex_no_snoop(struct e1000_hw *hw,
+					 uint32_t no_snoop);
+static int32_t e1000_verify_write_ich8_byte(struct e1000_hw *hw,
+					    uint32_t index, uint8_t byte);
+static int32_t e1000_write_eeprom_ich8(struct e1000_hw *hw, uint16_t offset,
+				       uint16_t words, uint16_t *data);
+static int32_t e1000_write_ich8_byte(struct e1000_hw *hw, uint32_t index,
+				     uint8_t data);
+static int32_t e1000_write_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr,
+				    uint16_t data);
+
 /* IGP cable length table */
 static const
 uint16_t e1000_igp_cable_length_table[IGP01E1000_AGC_LENGTH_TABLE_SIZE] =
@@ -3233,7 +3260,7 @@
     return data;
 }
 
-int32_t
+static int32_t
 e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask)
 {
     uint32_t swfw_sync = 0;
@@ -3277,7 +3304,7 @@
     return E1000_SUCCESS;
 }
 
-void
+static void
 e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask)
 {
     uint32_t swfw_sync;
@@ -3575,7 +3602,7 @@
     return E1000_SUCCESS;
 }
 
-int32_t
+static int32_t
 e1000_read_kmrn_reg(struct e1000_hw *hw,
                     uint32_t reg_addr,
                     uint16_t *data)
@@ -3608,7 +3635,7 @@
     return E1000_SUCCESS;
 }
 
-int32_t
+static int32_t
 e1000_write_kmrn_reg(struct e1000_hw *hw,
                      uint32_t reg_addr,
                      uint16_t data)
@@ -3839,7 +3866,7 @@
 *
 * hw - struct containing variables accessed by shared code
 ******************************************************************************/
-int32_t
+static int32_t
 e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw)
 {
     int32_t ret_val;
@@ -4086,7 +4113,7 @@
 * hw - Struct containing variables accessed by shared code
 * phy_info - PHY information structure
 ******************************************************************************/
-int32_t
+static int32_t
 e1000_phy_ife_get_info(struct e1000_hw *hw,
                        struct e1000_phy_info *phy_info)
 {
@@ -5643,6 +5670,7 @@
  * for the first 15 multicast addresses, and hashes the rest into the
  * multicast table.
  *****************************************************************************/
+#if 0
 void
 e1000_mc_addr_list_update(struct e1000_hw *hw,
                           uint8_t *mc_addr_list,
@@ -5719,6 +5747,7 @@
     }
     DEBUGOUT("MC Update Complete\n");
 }
+#endif  /*  0  */
 
 /******************************************************************************
  * Hashes an address to determine its location in the multicast table
@@ -6587,6 +6616,7 @@
  * hw - Struct containing variables accessed by shared code
  * offset - offset to read from
  *****************************************************************************/
+#if 0
 uint32_t
 e1000_read_reg_io(struct e1000_hw *hw,
                   uint32_t offset)
@@ -6597,6 +6627,7 @@
     e1000_io_write(hw, io_addr, offset);
     return e1000_io_read(hw, io_data);
 }
+#endif  /*  0  */
 
 /******************************************************************************
  * Writes a value to one of the devices registers using port I/O (as opposed to
@@ -7909,6 +7940,7 @@
  * returns: - none.
  *
  ***************************************************************************/
+#if 0
 void
 e1000_enable_pciex_master(struct e1000_hw *hw)
 {
@@ -7923,6 +7955,7 @@
     ctrl &= ~E1000_CTRL_GIO_MASTER_DISABLE;
     E1000_WRITE_REG(hw, CTRL, ctrl);
 }
+#endif  /*  0  */
 
 /*******************************************************************************
  *
@@ -8148,7 +8181,7 @@
  *            E1000_SUCCESS at any other case.
  *
  ***************************************************************************/
-int32_t
+static int32_t
 e1000_get_software_semaphore(struct e1000_hw *hw)
 {
     int32_t timeout = hw->eeprom.word_size + 1;
@@ -8183,7 +8216,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  ***************************************************************************/
-void
+static void
 e1000_release_software_semaphore(struct e1000_hw *hw)
 {
     uint32_t swsm;
@@ -8265,7 +8298,7 @@
  * returns: E1000_SUCCESS
  *
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_set_pci_ex_no_snoop(struct e1000_hw *hw, uint32_t no_snoop)
 {
     uint32_t gcr_reg = 0;
@@ -8306,7 +8339,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  ***************************************************************************/
-int32_t
+static int32_t
 e1000_get_software_flag(struct e1000_hw *hw)
 {
     int32_t timeout = PHY_CFG_TIMEOUT;
@@ -8345,7 +8378,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  ***************************************************************************/
-void
+static void
 e1000_release_software_flag(struct e1000_hw *hw)
 {
     uint32_t extcnf_ctrl;
@@ -8369,6 +8402,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  ***************************************************************************/
+#if 0
 int32_t
 e1000_ife_disable_dynamic_power_down(struct e1000_hw *hw)
 {
@@ -8388,6 +8422,7 @@
 
     return ret_val;
 }
+#endif  /*  0  */
 
 /***************************************************************************
  *
@@ -8397,6 +8432,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  ***************************************************************************/
+#if 0
 int32_t
 e1000_ife_enable_dynamic_power_down(struct e1000_hw *hw)
 {
@@ -8416,6 +8452,7 @@
 
     return ret_val;
 }
+#endif  /*  0  */
 
 /******************************************************************************
  * Reads a 16 bit word or words from the EEPROM using the ICH8's flash access
@@ -8426,7 +8463,7 @@
  * data - word read from the EEPROM
  * words - number of words to read
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_read_eeprom_ich8(struct e1000_hw *hw, uint16_t offset, uint16_t words,
                        uint16_t *data)
 {
@@ -8482,7 +8519,7 @@
  * words - number of words to write
  * data - words to write to the EEPROM
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_write_eeprom_ich8(struct e1000_hw *hw, uint16_t offset, uint16_t words,
                         uint16_t *data)
 {
@@ -8529,7 +8566,7 @@
  *
  * hw - The pointer to the hw structure
  ****************************************************************************/
-int32_t
+static int32_t
 e1000_ich8_cycle_init(struct e1000_hw *hw)
 {
     union ich8_hws_flash_status hsfsts;
@@ -8596,7 +8633,7 @@
  *
  * hw - The pointer to the hw structure
  ****************************************************************************/
-int32_t
+static int32_t
 e1000_ich8_flash_cycle(struct e1000_hw *hw, uint32_t timeout)
 {
     union ich8_hws_flash_ctrl hsflctl;
@@ -8631,7 +8668,7 @@
  * size - Size of data to read, 1=byte 2=word
  * data - Pointer to the word to store the value read.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_read_ich8_data(struct e1000_hw *hw, uint32_t index,
                      uint32_t size, uint16_t* data)
 {
@@ -8710,7 +8747,7 @@
  * size - Size of data to read, 1=byte 2=word
  * data - The byte(s) to write to the NVM.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_write_ich8_data(struct e1000_hw *hw, uint32_t index, uint32_t size,
                       uint16_t data)
 {
@@ -8785,7 +8822,7 @@
  * index - The index of the byte to read.
  * data - Pointer to a byte to store the value read.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_read_ich8_byte(struct e1000_hw *hw, uint32_t index, uint8_t* data)
 {
     int32_t status = E1000_SUCCESS;
@@ -8808,7 +8845,7 @@
  * index - The index of the byte to write.
  * byte - The byte to write to the NVM.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_verify_write_ich8_byte(struct e1000_hw *hw, uint32_t index, uint8_t byte)
 {
     int32_t error = E1000_SUCCESS;
@@ -8839,7 +8876,7 @@
  * index - The index of the byte to read.
  * data - The byte to write to the NVM.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_write_ich8_byte(struct e1000_hw *hw, uint32_t index, uint8_t data)
 {
     int32_t status = E1000_SUCCESS;
@@ -8857,7 +8894,7 @@
  * index - The starting byte index of the word to read.
  * data - Pointer to a word to store the value read.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_read_ich8_word(struct e1000_hw *hw, uint32_t index, uint16_t *data)
 {
     int32_t status = E1000_SUCCESS;
@@ -8872,6 +8909,7 @@
  * index - The starting byte index of the word to read.
  * data - The word to write to the NVM.
  *****************************************************************************/
+#if 0
 int32_t
 e1000_write_ich8_word(struct e1000_hw *hw, uint32_t index, uint16_t data)
 {
@@ -8879,6 +8917,7 @@
     status = e1000_write_ich8_data(hw, index, 2, data);
     return status;
 }
+#endif  /*  0  */
 
 /******************************************************************************
  * Erases the bank specified. Each bank is a 4k block. Segments are 0 based.
@@ -8887,7 +8926,7 @@
  * hw - pointer to e1000_hw structure
  * segment - 0 for first segment, 1 for second segment, etc.
  *****************************************************************************/
-int32_t
+static int32_t
 e1000_erase_ich8_4k_segment(struct e1000_hw *hw, uint32_t segment)
 {
     union ich8_hws_flash_status hsfsts;
@@ -8984,6 +9023,7 @@
  * hw: Struct containing variables accessed by shared code
  *
  *****************************************************************************/
+#if 0
 int32_t
 e1000_duplex_reversal(struct e1000_hw *hw)
 {
@@ -9012,8 +9052,9 @@
 
     return ret_val;
 }
+#endif  /*  0  */
 
-int32_t
+static int32_t
 e1000_init_lcd_from_nvm_config_region(struct e1000_hw *hw,
                                       uint32_t cnf_base_addr, uint32_t cnf_size)
 {
@@ -9047,7 +9088,7 @@
 }
 
 
-int32_t
+static int32_t
 e1000_init_lcd_from_nvm(struct e1000_hw *hw)
 {
     uint32_t reg_data, cnf_base_addr, cnf_size, ret_val, loop;
--- linux-2.6.17-mm6-full/drivers/net/e1000/e1000_main.c.old	2006-07-06 21:57:45.000000000 +0200
+++ linux-2.6.17-mm6-full/drivers/net/e1000/e1000_main.c	2006-07-06 21:58:01.000000000 +0200
@@ -4387,11 +4387,13 @@
 	pci_write_config_word(adapter->pdev, reg, *value);
 }
 
+#if 0
 uint32_t
 e1000_io_read(struct e1000_hw *hw, unsigned long port)
 {
 	return inl(port);
 }
+#endif  /*  0  */
 
 void
 e1000_io_write(struct e1000_hw *hw, unsigned long port, uint32_t value)


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

* Re: 2.6.17-mm6
       [not found] <20060703030355.420c7155.akpm@osdl.org>
       [not found] ` <200607042153.31848.rjw@sisk.pl>
  2006-07-06 20:37 ` [-mm patch] drivers/net/e1000/: possible cleanups Adrian Bunk
@ 2006-07-07 15:24 ` Reuben Farrelly
  2 siblings, 0 replies; 6+ messages in thread
From: Reuben Farrelly @ 2006-07-07 15:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, netdev



On 3/07/2006 10:03 p.m., Andrew Morton wrote:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17/2.6.17-mm6/
> 
> 
> - A major update to the e1000 driver.
> 
> - 1394 updates

Some minor breakage in the e1000...

Fedora Core release 5.90 (Test)
Kernel 2.6.17-mm6 on an x86_64

tornado.reub.net login: e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
   Tx Queue             <0>
   TDH                  <a>
   TDT                  <1c>
   next_to_use          <1c>
   next_to_clean        <8>
buffer_info[next_to_clean]
   time_stamp           <100027f1a>
   next_to_watch        <a>
   jiffies              <1000281d4>
   next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
   Tx Queue             <0>
   TDH                  <a>
   TDT                  <1c>
   next_to_use          <1c>
   next_to_clean        <8>
buffer_info[next_to_clean]
   time_stamp           <100027f1a>
   next_to_watch        <a>
   jiffies              <1000283c8>
   next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
   Tx Queue             <0>
   TDH                  <a>
   TDT                  <1c>
   next_to_use          <1c>
   next_to_clean        <8>
buffer_info[next_to_clean]
   time_stamp           <100027f1a>
   next_to_watch        <a>
   jiffies              <1000285bc>
   next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
   Tx Queue             <0>
   TDH                  <a>
   TDT                  <1c>
   next_to_use          <1c>
   next_to_clean        <8>
buffer_info[next_to_clean]
   time_stamp           <100027f1a>
   next_to_watch        <a>
   jiffies              <1000287b0>
   next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
   Tx Queue             <0>
   TDH                  <a>
   TDT                  <1c>
   next_to_use          <1c>
   next_to_clean        <8>
buffer_info[next_to_clean]
   time_stamp           <100027f1a>
   next_to_watch        <a>
   jiffies              <1000289a4>
   next_to_watch.status <0>


A look through my switch logs and kernel logs over the last few days shows these 
messages and layer 2/link down disconnections every few hours or so, but of very 
short duration (I hadn't noticed until now).

This output above was under virtually no load.

Both the e1000 and switch port on the other end are doing RX and TX flow control.

The controller is a built in chip on an Intel D945GNT board.

01:00.0 Ethernet controller: Intel Corporation 82573V Gigabit Ethernet 
Controller (Copper) (rev 03)
         Subsystem: Intel Corporation Unknown device 3094
         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B-
         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR-
         Latency: 0, Cache Line Size: 64 bytes
         Interrupt: pin A routed to IRQ 313
         Region 0: Memory at 48000000 (32-bit, non-prefetchable) [size=128K]
         Region 2: I/O ports at 2000 [size=32]
         Capabilities: [c8] Power Management version 2
                 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA 
PME(D0+,D1-,D2-,D3hot+,D3cold+)
                 Status: D0 PME-Enable- DSel=0 DScale=1 PME-
         Capabilities: [d0] Message Signalled Interrupts: 64bit+ Queue=0/0 Enable+
                 Address: 00000000fee0100c  Data: 4142
         Capabilities: [e0] Express Endpoint IRQ 0
                 Device: Supported: MaxPayload 256 bytes, PhantFunc 0, ExtTag-
                 Device: Latency L0s <512ns, L1 <64us
                 Device: AtnBtn- AtnInd- PwrInd-
                 Device: Errors: Correctable- Non-Fatal- Fatal- Unsupported-
                 Device: RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                 Device: MaxPayload 128 bytes, MaxReadReq 512 bytes
                 Link: Supported Speed 2.5Gb/s, Width x1, ASPM unknown, Port 0
                 Link: Latency L0s <128ns, L1 <64us
                 Link: ASPM Disabled RCB 64 bytes CommClk+ ExtSynch-
                 Link: Speed 2.5Gb/s, Width x1

[root@tornado log]# ethtool -i eth0
driver: e1000
version: 7.1.9-k2-NAPI
firmware-version: 1.0-5
bus-info: 0000:01:00.0
[root@tornado log]#

Where can I go from here to help debug this further?

reuben

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

end of thread, other threads:[~2006-07-07 15:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20060703030355.420c7155.akpm@osdl.org>
     [not found] ` <200607042153.31848.rjw@sisk.pl>
2006-07-04 20:01   ` 2.6.17-mm6 Arjan van de Ven
2006-07-05 10:27     ` 2.6.17-mm6 Stefan Richter
2006-07-05 10:36       ` 2.6.17-mm6 Stefan Richter
2006-07-05 11:13         ` 2.6.17-mm6 Ingo Molnar
2006-07-06 20:37 ` [-mm patch] drivers/net/e1000/: possible cleanups Adrian Bunk
2006-07-07 15:24 ` 2.6.17-mm6 Reuben Farrelly

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).