* [net-next-2.6 PATCH 1/3] e1000: use work queues
@ 2010-09-23 4:22 Jeff Kirsher
2010-09-23 4:22 ` [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload Jeff Kirsher
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jeff Kirsher @ 2010-09-23 4:22 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Jesse Brandeburg, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
E1000 is using several timers that in a follow on patch
will need to acquire the rtnl_lock in order to be safe.
This patch moves the timer bodies into work queues which
will allow the next patch to add rtnl_lock.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000/e1000.h | 3 +++
drivers/net/e1000/e1000_main.c | 25 ++++++++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 99288b9..a881dd0 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -310,6 +310,9 @@ struct e1000_adapter {
int need_ioport;
bool discarding;
+
+ struct work_struct fifo_stall_task;
+ struct work_struct phy_info_task;
};
enum e1000_state_t {
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index a1dacea..5b4c6c0 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -123,8 +123,10 @@ static void e1000_clean_rx_ring(struct e1000_adapter *adapter,
struct e1000_rx_ring *rx_ring);
static void e1000_set_rx_mode(struct net_device *netdev);
static void e1000_update_phy_info(unsigned long data);
+static void e1000_update_phy_info_task(struct work_struct *work);
static void e1000_watchdog(unsigned long data);
static void e1000_82547_tx_fifo_stall(unsigned long data);
+static void e1000_82547_tx_fifo_stall_task(struct work_struct *work);
static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
struct net_device *netdev);
static struct net_device_stats * e1000_get_stats(struct net_device *netdev);
@@ -1047,7 +1049,9 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
adapter->phy_info_timer.function = e1000_update_phy_info;
adapter->phy_info_timer.data = (unsigned long)adapter;
+ INIT_WORK(&adapter->fifo_stall_task, e1000_82547_tx_fifo_stall_task);
INIT_WORK(&adapter->reset_task, e1000_reset_task);
+ INIT_WORK(&adapter->phy_info_task, e1000_update_phy_info_task);
e1000_check_options(adapter);
@@ -2234,6 +2238,14 @@ static void e1000_set_rx_mode(struct net_device *netdev)
static void e1000_update_phy_info(unsigned long data)
{
struct e1000_adapter *adapter = (struct e1000_adapter *)data;
+ schedule_work(&adapter->phy_info_task);
+}
+
+static void e1000_update_phy_info_task(struct work_struct *work)
+{
+ struct e1000_adapter *adapter = container_of(work,
+ struct e1000_adapter,
+ phy_info_task);
struct e1000_hw *hw = &adapter->hw;
e1000_phy_get_info(hw, &adapter->phy_info);
}
@@ -2242,10 +2254,21 @@ static void e1000_update_phy_info(unsigned long data)
* e1000_82547_tx_fifo_stall - Timer Call-back
* @data: pointer to adapter cast into an unsigned long
**/
-
static void e1000_82547_tx_fifo_stall(unsigned long data)
{
struct e1000_adapter *adapter = (struct e1000_adapter *)data;
+ schedule_work(&adapter->fifo_stall_task);
+}
+
+/**
+ * e1000_82547_tx_fifo_stall_task - task to complete work
+ * @work: work struct contained inside adapter struct
+ **/
+static void e1000_82547_tx_fifo_stall_task(struct work_struct *work)
+{
+ struct e1000_adapter *adapter = container_of(work,
+ struct e1000_adapter,
+ fifo_stall_task);
struct e1000_hw *hw = &adapter->hw;
struct net_device *netdev = adapter->netdev;
u32 tctl;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload
2010-09-23 4:22 [net-next-2.6 PATCH 1/3] e1000: use work queues Jeff Kirsher
@ 2010-09-23 4:22 ` Jeff Kirsher
2010-09-23 21:34 ` David Miller
2010-09-23 4:23 ` [net-next-2.6 PATCH 3/3] e1000: use GRO for receive Jeff Kirsher
2010-09-23 21:34 ` [net-next-2.6 PATCH 1/3] e1000: use work queues David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2010-09-23 4:22 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Jesse Brandeburg, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Net drivers in general have an issue where timers fired
by mod_timer or work threads with schedule_work are running
outside of the rtnl_lock.
With no other lock protection these routines are vulnerable
to races with driver unload or reset paths.
The longer term solution to this might be a redesign with
safer locks being taken in the driver to guarantee no
reentrance, but for now a safe and effective fix is
to take the rtnl_lock in these routines.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000/e1000_main.c | 20 +++++++++++++++++++-
1 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 5b4c6c0..c88439d 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -521,8 +521,21 @@ void e1000_down(struct e1000_adapter *adapter)
e1000_clean_all_rx_rings(adapter);
}
+void e1000_reinit_safe(struct e1000_adapter *adapter)
+{
+ while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
+ msleep(1);
+ rtnl_lock();
+ e1000_down(adapter);
+ e1000_up(adapter);
+ rtnl_unlock();
+ clear_bit(__E1000_RESETTING, &adapter->flags);
+}
+
void e1000_reinit_locked(struct e1000_adapter *adapter)
{
+ /* if rtnl_lock is not held the call path is bogus */
+ ASSERT_RTNL();
WARN_ON(in_interrupt());
while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
msleep(1);
@@ -2247,7 +2260,10 @@ static void e1000_update_phy_info_task(struct work_struct *work)
struct e1000_adapter,
phy_info_task);
struct e1000_hw *hw = &adapter->hw;
+
+ rtnl_lock();
e1000_phy_get_info(hw, &adapter->phy_info);
+ rtnl_unlock();
}
/**
@@ -2273,6 +2289,7 @@ static void e1000_82547_tx_fifo_stall_task(struct work_struct *work)
struct net_device *netdev = adapter->netdev;
u32 tctl;
+ rtnl_lock();
if (atomic_read(&adapter->tx_fifo_stall)) {
if ((er32(TDT) == er32(TDH)) &&
(er32(TDFT) == er32(TDFH)) &&
@@ -2293,6 +2310,7 @@ static void e1000_82547_tx_fifo_stall_task(struct work_struct *work)
mod_timer(&adapter->tx_fifo_stall_timer, jiffies + 1);
}
}
+ rtnl_unlock();
}
bool e1000_has_link(struct e1000_adapter *adapter)
@@ -3160,7 +3178,7 @@ static void e1000_reset_task(struct work_struct *work)
struct e1000_adapter *adapter =
container_of(work, struct e1000_adapter, reset_task);
- e1000_reinit_locked(adapter);
+ e1000_reinit_safe(adapter);
}
/**
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [net-next-2.6 PATCH 3/3] e1000: use GRO for receive
2010-09-23 4:22 [net-next-2.6 PATCH 1/3] e1000: use work queues Jeff Kirsher
2010-09-23 4:22 ` [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload Jeff Kirsher
@ 2010-09-23 4:23 ` Jeff Kirsher
2010-09-23 21:34 ` David Miller
2010-09-23 21:34 ` [net-next-2.6 PATCH 1/3] e1000: use work queues David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2010-09-23 4:23 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Jesse Brandeburg, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
E1000 can benefit from calling the GRO receive functions.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000/e1000_main.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index c88439d..796523f 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3664,13 +3664,14 @@ static void e1000_consume_page(struct e1000_buffer *bi, struct sk_buff *skb,
static void e1000_receive_skb(struct e1000_adapter *adapter, u8 status,
__le16 vlan, struct sk_buff *skb)
{
- if (unlikely(adapter->vlgrp && (status & E1000_RXD_STAT_VP))) {
- vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
- le16_to_cpu(vlan) &
- E1000_RXD_SPC_VLAN_MASK);
- } else {
- netif_receive_skb(skb);
- }
+ skb->protocol = eth_type_trans(skb, adapter->netdev);
+
+ if ((unlikely(adapter->vlgrp && (status & E1000_RXD_STAT_VP))))
+ vlan_gro_receive(&adapter->napi, adapter->vlgrp,
+ le16_to_cpu(vlan) & E1000_RXD_SPC_VLAN_MASK,
+ skb);
+ else
+ napi_gro_receive(&adapter->napi, skb);
}
/**
@@ -3828,8 +3829,6 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_adapter *adapter,
goto next_desc;
}
- skb->protocol = eth_type_trans(skb, netdev);
-
e1000_receive_skb(adapter, status, rx_desc->special, skb);
next_desc:
@@ -3992,8 +3991,6 @@ static bool e1000_clean_rx_irq(struct e1000_adapter *adapter,
((u32)(rx_desc->errors) << 24),
le16_to_cpu(rx_desc->csum), skb);
- skb->protocol = eth_type_trans(skb, netdev);
-
e1000_receive_skb(adapter, status, rx_desc->special, skb);
next_desc:
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [net-next-2.6 PATCH 1/3] e1000: use work queues
2010-09-23 4:22 [net-next-2.6 PATCH 1/3] e1000: use work queues Jeff Kirsher
2010-09-23 4:22 ` [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload Jeff Kirsher
2010-09-23 4:23 ` [net-next-2.6 PATCH 3/3] e1000: use GRO for receive Jeff Kirsher
@ 2010-09-23 21:34 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:22:17 -0700
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> E1000 is using several timers that in a follow on patch
> will need to acquire the rtnl_lock in order to be safe.
>
> This patch moves the timer bodies into work queues which
> will allow the next patch to add rtnl_lock.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload
2010-09-23 4:22 ` [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload Jeff Kirsher
@ 2010-09-23 21:34 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:22:42 -0700
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> Net drivers in general have an issue where timers fired
> by mod_timer or work threads with schedule_work are running
> outside of the rtnl_lock.
>
> With no other lock protection these routines are vulnerable
> to races with driver unload or reset paths.
>
> The longer term solution to this might be a redesign with
> safer locks being taken in the driver to guarantee no
> reentrance, but for now a safe and effective fix is
> to take the rtnl_lock in these routines.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next-2.6 PATCH 3/3] e1000: use GRO for receive
2010-09-23 4:23 ` [net-next-2.6 PATCH 3/3] e1000: use GRO for receive Jeff Kirsher
@ 2010-09-23 21:34 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:23:05 -0700
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> E1000 can benefit from calling the GRO receive functions.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-23 21:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-23 4:22 [net-next-2.6 PATCH 1/3] e1000: use work queues Jeff Kirsher
2010-09-23 4:22 ` [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload Jeff Kirsher
2010-09-23 21:34 ` David Miller
2010-09-23 4:23 ` [net-next-2.6 PATCH 3/3] e1000: use GRO for receive Jeff Kirsher
2010-09-23 21:34 ` David Miller
2010-09-23 21:34 ` [net-next-2.6 PATCH 1/3] e1000: use work queues David Miller
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).