* Re: [PATCH net] af_packet: use spin_lock_bh() for sk_buff_head
From: Veaceslav Falico @ 2013-11-28 7:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, jstancek, David S. Miller, Daniel Borkmann,
Willem de Bruijn, Phil Sutter, Eric Dumazet
In-Reply-To: <1385569075.5352.12.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 27, 2013 at 08:17:55AM -0800, Eric Dumazet wrote:
>On Wed, 2013-11-27 at 13:57 +0100, Veaceslav Falico wrote:
>> While running a debug kernel, a warning of inconsistent lock state showed
>> up for sk->sk_receive_queue->lock between packet_set_ring() (actually,
>> static prb_shutdown_retire_blk_timer(), which does spin_lock()) and
>> sock_queue_rcv_skb(), which does spin_lock_irqsave().
>>
>> Fix this by converting the spin_lock() used in
>> prb_shutdown_retire_blk_timer() to spin_lock_bh(), as it is used in
>> packet_set_ring().
>
>Patch is good, but description is slightly misleading.
Yep, thank you, will reword and resubmit.
>
>Problem is not because sock_queue_rcv_skb() uses spin_lock_irqsave().
>
>Problem is prb_retire_rx_blk_timer_expired() is called from softirq
>context.
>
>So if we do not block BH in prb_shutdown_retire_blk_timer() :
>
>spin_lock(&rb_queue->lock);
>pkc->delete_blk_timer = 1;
>spin_unlock(&rb_queue->lock);
>
>Timer could fire right in the middle and would spin forever trying
>to acquire the same lock.
>
>Using spin_lock_bh() prevents timer being fired on this cpu.
>
>Thanks !
>
>
^ permalink raw reply
* Re: 3.13.0-rc1+ INFO: inconsistent lock state
From: Hannes Frederic Sowa @ 2013-11-28 7:18 UTC (permalink / raw)
To: 허종만
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <13025162.118881385612276530.JavaMail.weblogic@epv6ml01>
On Thu, Nov 28, 2013 at 04:17:56AM +0000, 허종만 wrote:
> With current linus git (HEAD 8ae516aa), I got following kernel message.
> This is slightly different from what I had reported recently (14th Nov).
I guess the prolem is that the STATS _BH functions are to be used if they
are executed in bottom half and maybe people expect it to protect against
bottom half. We seem to have some more of this problems in udp_sendmsg
(this trace) and ping_sendmsg, maybe in tcp, too. The non-_BH functions
actually disable bh.
I'll check for more prolems and will post a patch.
Thanks for reporting,
Hannes
^ permalink raw reply
* [PATCH 2/2] net: emaclite: add barriers to support Xilinx Zynq platform
From: Srikanth Thokala @ 2013-11-28 7:29 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: davem, Michal Simek, Srikanth Thokala
In-Reply-To: <1385623797-26034-1-git-send-email-sthokal@xilinx.com>
This patch adds barriers at appropriate places to ensure the driver
works on Xilinx Zynq ARM-based SoC platform.
Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 3a693a1..31b8b6c 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -220,6 +220,7 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
to_u16_ptr = (u16 *)&align_buffer;
*to_u16_ptr++ = *from_u16_ptr++;
*to_u16_ptr++ = *from_u16_ptr++;
+ wmb();
/* Output a word */
*to_u32_ptr++ = align_buffer;
@@ -236,6 +237,7 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
for (; length > 0; length--)
*to_u8_ptr++ = *from_u8_ptr++;
+ wmb();
*to_u32_ptr = align_buffer;
}
}
--
1.8.2.GIT
^ permalink raw reply related
* [PATCH 1/2] net: emaclite: Remove unnecessary code that enables/disables interrupts on PONG buffers
From: Srikanth Thokala @ 2013-11-28 7:29 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: davem, Michal Simek, Srikanth Thokala
There are no specific interrupts for the PONG buffer on both
transmit and receive side, same interrupt is valid for both
buffers. So, this patch removes this code.
Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 38 ---------------------------
1 file changed, 38 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index a7c02f4..3a693a1 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -164,26 +164,9 @@ static void xemaclite_enable_interrupts(struct net_local *drvdata)
__raw_writel(reg_data | XEL_TSR_XMIT_IE_MASK,
drvdata->base_addr + XEL_TSR_OFFSET);
- /* Enable the Tx interrupts for the second Buffer if
- * configured in HW */
- if (drvdata->tx_ping_pong != 0) {
- reg_data = __raw_readl(drvdata->base_addr +
- XEL_BUFFER_OFFSET + XEL_TSR_OFFSET);
- __raw_writel(reg_data | XEL_TSR_XMIT_IE_MASK,
- drvdata->base_addr + XEL_BUFFER_OFFSET +
- XEL_TSR_OFFSET);
- }
-
/* Enable the Rx interrupts for the first buffer */
__raw_writel(XEL_RSR_RECV_IE_MASK, drvdata->base_addr + XEL_RSR_OFFSET);
- /* Enable the Rx interrupts for the second Buffer if
- * configured in HW */
- if (drvdata->rx_ping_pong != 0) {
- __raw_writel(XEL_RSR_RECV_IE_MASK, drvdata->base_addr +
- XEL_BUFFER_OFFSET + XEL_RSR_OFFSET);
- }
-
/* Enable the Global Interrupt Enable */
__raw_writel(XEL_GIER_GIE_MASK, drvdata->base_addr + XEL_GIER_OFFSET);
}
@@ -207,31 +190,10 @@ static void xemaclite_disable_interrupts(struct net_local *drvdata)
__raw_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK),
drvdata->base_addr + XEL_TSR_OFFSET);
- /* Disable the Tx interrupts for the second Buffer
- * if configured in HW */
- if (drvdata->tx_ping_pong != 0) {
- reg_data = __raw_readl(drvdata->base_addr + XEL_BUFFER_OFFSET +
- XEL_TSR_OFFSET);
- __raw_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK),
- drvdata->base_addr + XEL_BUFFER_OFFSET +
- XEL_TSR_OFFSET);
- }
-
/* Disable the Rx interrupts for the first buffer */
reg_data = __raw_readl(drvdata->base_addr + XEL_RSR_OFFSET);
__raw_writel(reg_data & (~XEL_RSR_RECV_IE_MASK),
drvdata->base_addr + XEL_RSR_OFFSET);
-
- /* Disable the Rx interrupts for the second buffer
- * if configured in HW */
- if (drvdata->rx_ping_pong != 0) {
-
- reg_data = __raw_readl(drvdata->base_addr + XEL_BUFFER_OFFSET +
- XEL_RSR_OFFSET);
- __raw_writel(reg_data & (~XEL_RSR_RECV_IE_MASK),
- drvdata->base_addr + XEL_BUFFER_OFFSET +
- XEL_RSR_OFFSET);
- }
}
/**
--
1.8.2.GIT
^ permalink raw reply related
* [net 0/8][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to igb, e1000 and ixgbe.
Akeem provides a igb fix where WOL was being reported as supported on
some ethernet devices which did not have that capability.
Carolyn provides a igb fix to add a call to dev_close if the queue
reinit fails in order to make it clear to the user that the device is
down.
Yanjun provides a fix for e1000 which is similar to a previous fix
for e1000e commit bb9e44d0d0f4 ("e1000e: prevent oops when adapter is
being closed and reset simultaneously"), where the same issue was
observed on the older e1000 cards.
Vladimir Davydov provides 2 e1000 fixes. The first fixes a lockdep
warning e1000_down() tries to synchronously cancel e1000 auxiliary
works (reset_task, watchdog_task, phy_info_task and fifo_stall_task)
which take adapter->mutex in their handlers. The second patch is to
fix a possible race condition where reset_task() would be running
after adapter down.
John provides 2 fixes for ixgbe. First turns ixgbe_fwd_ring_down
to static and the second disables NETIF_F_HW_L2FW_DOFFLOAD by default
because it allows upper layer net devices to use queues in the hardware
to directly submit and receive skbs.
Mark Rustad provides a single patch for ixgbe to make
ixgbe_identify_qsfp_module_generic static to resolve compile
warnings.
The following are changes since commit 2c7a9dc1641664173211c4ebc5db510a08684c46:
be2net: Avoid programming permenant MAC by BE3-R VFs
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Akeem G Abodunrin (1):
igb: Fixed Wake On LAN support
Carolyn Wyborny (1):
igb: Update queue reinit function to call dev_close when init of
queues fails
John Fastabend (2):
ixgbe: ixgbe_fwd_ring_down needs to be static
ixgbe: turn NETIF_F_HW_L2FW_DOFFLOAD off by default
Mark Rustad (1):
ixgbe: Make ixgbe_identify_qsfp_module_generic static
Vladimir Davydov (2):
e1000: fix lockdep warning in e1000_reset_task
e1000: fix possible reset_task running after adapter down
yzhu1 (1):
e1000: prevent oops when adapter is being closed and reset
simultaneously
drivers/net/ethernet/intel/e1000/e1000.h | 7 +++-
drivers/net/ethernet/intel/e1000/e1000_main.c | 60 ++++++++++-----------------
drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 3 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 1 -
7 files changed, 39 insertions(+), 49 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [net 3/8] e1000: prevent oops when adapter is being closed and reset simultaneously
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem; +Cc: yzhu1, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: yzhu1 <yanjun.zhu@windriver.com>
This change is based on a similar change made to e1000e support in
commit bb9e44d0d0f4 ("e1000e: prevent oops when adapter is being closed
and reset simultaneously"). The same issue has also been observed
on the older e1000 cards.
Here, we have increased the RESET_COUNT value to 50 because there are too
many accesses to e1000 nic on stress tests to e1000 nic, it is not enough
to set RESET_COUT 25. Experimentation has shown that it is enough to set
RESET_COUNT 50.
Signed-off-by: yzhu1 <yanjun.zhu@windriver.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000.h | 5 +++++
drivers/net/ethernet/intel/e1000/e1000_main.c | 9 +++++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
index 58c1472..e4093d1 100644
--- a/drivers/net/ethernet/intel/e1000/e1000.h
+++ b/drivers/net/ethernet/intel/e1000/e1000.h
@@ -83,6 +83,11 @@ struct e1000_adapter;
#define E1000_MAX_INTR 10
+/*
+ * Count for polling __E1000_RESET condition every 10-20msec.
+ */
+#define E1000_CHECK_RESET_COUNT 50
+
/* TX/RX descriptor defines */
#define E1000_DEFAULT_TXD 256
#define E1000_MAX_TXD 256
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index e386228..c0f5217 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1440,6 +1440,10 @@ static int e1000_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
+ int count = E1000_CHECK_RESET_COUNT;
+
+ while (test_bit(__E1000_RESETTING, &adapter->flags) && count--)
+ usleep_range(10000, 20000);
WARN_ON(test_bit(__E1000_RESETTING, &adapter->flags));
e1000_down(adapter);
@@ -4963,6 +4967,11 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
netif_device_detach(netdev);
if (netif_running(netdev)) {
+ int count = E1000_CHECK_RESET_COUNT;
+
+ while (test_bit(__E1000_RESETTING, &adapter->flags) && count--)
+ usleep_range(10000, 20000);
+
WARN_ON(test_bit(__E1000_RESETTING, &adapter->flags));
e1000_down(adapter);
}
--
1.8.3.1
^ permalink raw reply related
* [net 2/8] igb: Fixed Wake On LAN support
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem; +Cc: Akeem G Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
This patch fixes Wake on LAN being reported as supported on some Ethernet
ports, in contrary to Hardware capability.
Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index b0f3666..c3143da 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2062,14 +2062,15 @@ static void igb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct igb_adapter *adapter = netdev_priv(netdev);
- wol->supported = WAKE_UCAST | WAKE_MCAST |
- WAKE_BCAST | WAKE_MAGIC |
- WAKE_PHY;
wol->wolopts = 0;
if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))
return;
+ wol->supported = WAKE_UCAST | WAKE_MCAST |
+ WAKE_BCAST | WAKE_MAGIC |
+ WAKE_PHY;
+
/* apply any specific unsupported masks here */
switch (adapter->hw.device_id) {
default:
--
1.8.3.1
^ permalink raw reply related
* [net 6/8] ixgbe: ixgbe_fwd_ring_down needs to be static
From: Jeff Kirsher @ 2013-11-28 7:36 UTC (permalink / raw)
To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
When compiling with -Wstrict-prototypes gcc catches a static
I missed.
./ixgbe_main.c:4254: warning: no previous prototype for 'ixgbe_fwd_ring_down'
Reported-by: Phillip Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0c55079..ad9d670 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4251,8 +4251,8 @@ static void ixgbe_disable_fwd_ring(struct ixgbe_fwd_adapter *vadapter,
rx_ring->l2_accel_priv = NULL;
}
-int ixgbe_fwd_ring_down(struct net_device *vdev,
- struct ixgbe_fwd_adapter *accel)
+static int ixgbe_fwd_ring_down(struct net_device *vdev,
+ struct ixgbe_fwd_adapter *accel)
{
struct ixgbe_adapter *adapter = accel->real_adapter;
unsigned int rxbase = accel->rx_base_queue;
--
1.8.3.1
^ permalink raw reply related
* [net 1/8] igb: Update queue reinit function to call dev_close when init of queues fails
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Carolyn Wyborny <carolyn.wyborny@intel.com>
This patch adds a call to dev_close if the queue reinit fails in order
to make clearer to the user that the device is down.
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 025e5f4..5a64aa6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7846,6 +7846,7 @@ int igb_reinit_queues(struct igb_adapter *adapter)
if (igb_init_interrupt_scheme(adapter, true)) {
dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
+ dev_close(netdev);
return -ENOMEM;
}
--
1.8.3.1
^ permalink raw reply related
* [net 7/8] ixgbe: turn NETIF_F_HW_L2FW_DOFFLOAD off by default
From: Jeff Kirsher @ 2013-11-28 7:36 UTC (permalink / raw)
To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
NETIF_F_HW_L2FW_DOFFLOAD allows upper layer net devices such
as macvlan to use queues in the hardware to directly submit and
receive skbs.
This creates a subtle change in the datapath though. One change
being the skb may no longer use the root devices qdisc.
Because users may not expect this we can't enable the feature
by default unless the hardware can offload all the software
functionality above it. So for now disable it by default and
let users opt in.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ad9d670..cc06854 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7986,10 +7986,9 @@ skip_sriov:
NETIF_F_TSO |
NETIF_F_TSO6 |
NETIF_F_RXHASH |
- NETIF_F_RXCSUM |
- NETIF_F_HW_L2FW_DOFFLOAD;
+ NETIF_F_RXCSUM;
- netdev->hw_features = netdev->features;
+ netdev->hw_features = netdev->features | NETIF_F_HW_L2FW_DOFFLOAD;
switch (adapter->hw.mac.type) {
case ixgbe_mac_82599EB:
--
1.8.3.1
^ permalink raw reply related
* [net 5/8] e1000: fix possible reset_task running after adapter down
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem
Cc: Vladimir Davydov, netdev, gospo, sassmann, Tushar Dave,
Patrick McHardy, Vladimir Davydov, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Vladimir Davydov <VDavydov@parallels.com>
On e1000_down(), we should ensure every asynchronous work is canceled
before proceeding. Since the watchdog_task can schedule other works
apart from itself, it should be stopped first, but currently it is
stopped after the reset_task. This can result in the following race
leading to the reset_task running after the module unload:
e1000_down_and_stop(): e1000_watchdog():
---------------------- -----------------
cancel_work_sync(reset_task)
schedule_work(reset_task)
cancel_delayed_work_sync(watchdog_task)
The patch moves cancel_delayed_work_sync(watchdog_task) at the beginning
of e1000_down_and_stop() thus ensuring the race is impossible.
Cc: Tushar Dave <tushar.n.dave@intel.com>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_main.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 619b0cb..46e6544 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -494,13 +494,20 @@ static void e1000_down_and_stop(struct e1000_adapter *adapter)
{
set_bit(__E1000_DOWN, &adapter->flags);
- /* Only kill reset task if adapter is not resetting */
- if (!test_bit(__E1000_RESETTING, &adapter->flags))
- cancel_work_sync(&adapter->reset_task);
-
cancel_delayed_work_sync(&adapter->watchdog_task);
+
+ /*
+ * Since the watchdog task can reschedule other tasks, we should cancel
+ * it first, otherwise we can run into the situation when a work is
+ * still running after the adapter has been turned down.
+ */
+
cancel_delayed_work_sync(&adapter->phy_info_task);
cancel_delayed_work_sync(&adapter->fifo_stall_task);
+
+ /* Only kill reset task if adapter is not resetting */
+ if (!test_bit(__E1000_RESETTING, &adapter->flags))
+ cancel_work_sync(&adapter->reset_task);
}
void e1000_down(struct e1000_adapter *adapter)
--
1.8.3.1
^ permalink raw reply related
* [net 4/8] e1000: fix lockdep warning in e1000_reset_task
From: Jeff Kirsher @ 2013-11-28 7:35 UTC (permalink / raw)
To: davem
Cc: Vladimir Davydov, netdev, gospo, sassmann, Tushar Dave,
Patrick McHardy, Vladimir Davydov, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Vladimir Davydov <VDavydov@parallels.com>
The patch fixes the following lockdep warning, which is 100%
reproducible on network restart:
======================================================
[ INFO: possible circular locking dependency detected ]
3.12.0+ #47 Tainted: GF
-------------------------------------------------------
kworker/1:1/27 is trying to acquire lock:
((&(&adapter->watchdog_task)->work)){+.+...}, at: [<ffffffff8108a5b0>] flush_work+0x0/0x70
but task is already holding lock:
(&adapter->mutex){+.+...}, at: [<ffffffffa0177c0a>] e1000_reset_task+0x4a/0xa0 [e1000]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&adapter->mutex){+.+...}:
[<ffffffff810bdb5d>] lock_acquire+0x9d/0x120
[<ffffffff816b8cbc>] mutex_lock_nested+0x4c/0x390
[<ffffffffa017233d>] e1000_watchdog+0x7d/0x5b0 [e1000]
[<ffffffff8108b972>] process_one_work+0x1d2/0x510
[<ffffffff8108ca80>] worker_thread+0x120/0x3a0
[<ffffffff81092c1e>] kthread+0xee/0x110
[<ffffffff816c3d7c>] ret_from_fork+0x7c/0xb0
-> #0 ((&(&adapter->watchdog_task)->work)){+.+...}:
[<ffffffff810bd9c0>] __lock_acquire+0x1710/0x1810
[<ffffffff810bdb5d>] lock_acquire+0x9d/0x120
[<ffffffff8108a5eb>] flush_work+0x3b/0x70
[<ffffffff8108b5d8>] __cancel_work_timer+0x98/0x140
[<ffffffff8108b693>] cancel_delayed_work_sync+0x13/0x20
[<ffffffffa0170cec>] e1000_down_and_stop+0x3c/0x60 [e1000]
[<ffffffffa01775b1>] e1000_down+0x131/0x220 [e1000]
[<ffffffffa0177c12>] e1000_reset_task+0x52/0xa0 [e1000]
[<ffffffff8108b972>] process_one_work+0x1d2/0x510
[<ffffffff8108ca80>] worker_thread+0x120/0x3a0
[<ffffffff81092c1e>] kthread+0xee/0x110
[<ffffffff816c3d7c>] ret_from_fork+0x7c/0xb0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&adapter->mutex);
lock((&(&adapter->watchdog_task)->work));
lock(&adapter->mutex);
lock((&(&adapter->watchdog_task)->work));
*** DEADLOCK ***
3 locks held by kworker/1:1/27:
#0: (events){.+.+.+}, at: [<ffffffff8108b906>] process_one_work+0x166/0x510
#1: ((&adapter->reset_task)){+.+...}, at: [<ffffffff8108b906>] process_one_work+0x166/0x510
#2: (&adapter->mutex){+.+...}, at: [<ffffffffa0177c0a>] e1000_reset_task+0x4a/0xa0 [e1000]
stack backtrace:
CPU: 1 PID: 27 Comm: kworker/1:1 Tainted: GF 3.12.0+ #47
Hardware name: System manufacturer System Product Name/P5B-VM SE, BIOS 0501 05/31/2007
Workqueue: events e1000_reset_task [e1000]
ffffffff820f6000 ffff88007b9dba98 ffffffff816b54a2 0000000000000002
ffffffff820f5e50 ffff88007b9dbae8 ffffffff810ba936 ffff88007b9dbac8
ffff88007b9dbb48 ffff88007b9d8f00 ffff88007b9d8780 ffff88007b9d8f00
Call Trace:
[<ffffffff816b54a2>] dump_stack+0x49/0x5f
[<ffffffff810ba936>] print_circular_bug+0x216/0x310
[<ffffffff810bd9c0>] __lock_acquire+0x1710/0x1810
[<ffffffff8108a5b0>] ? __flush_work+0x250/0x250
[<ffffffff810bdb5d>] lock_acquire+0x9d/0x120
[<ffffffff8108a5b0>] ? __flush_work+0x250/0x250
[<ffffffff8108a5eb>] flush_work+0x3b/0x70
[<ffffffff8108a5b0>] ? __flush_work+0x250/0x250
[<ffffffff8108b5d8>] __cancel_work_timer+0x98/0x140
[<ffffffff8108b693>] cancel_delayed_work_sync+0x13/0x20
[<ffffffffa0170cec>] e1000_down_and_stop+0x3c/0x60 [e1000]
[<ffffffffa01775b1>] e1000_down+0x131/0x220 [e1000]
[<ffffffffa0177c12>] e1000_reset_task+0x52/0xa0 [e1000]
[<ffffffff8108b972>] process_one_work+0x1d2/0x510
[<ffffffff8108b906>] ? process_one_work+0x166/0x510
[<ffffffff8108ca80>] worker_thread+0x120/0x3a0
[<ffffffff8108c960>] ? manage_workers+0x2c0/0x2c0
[<ffffffff81092c1e>] kthread+0xee/0x110
[<ffffffff81092b30>] ? __init_kthread_worker+0x70/0x70
[<ffffffff816c3d7c>] ret_from_fork+0x7c/0xb0
[<ffffffff81092b30>] ? __init_kthread_worker+0x70/0x70
== The issue background ==
The problem occurs, because e1000_down(), which is called under
adapter->mutex by e1000_reset_task(), tries to synchronously cancel
e1000 auxiliary works (reset_task, watchdog_task, phy_info_task,
fifo_stall_task), which take adapter->mutex in their handlers. So the
question is what does adapter->mutex protect there?
The adapter->mutex was introduced by commit 0ef4ee ("e1000: convert to
private mutex from rtnl") as a replacement for rtnl_lock() taken in the
asynchronous handlers. It targeted on fixing a similar lockdep warning
issued when e1000_down() was called under rtnl_lock(), and it fixed it,
but unfortunately it introduced the lockdep warning described above.
Anyway, that said the source of this bug is that the asynchronous works
were made to take rtnl_lock() some time ago, so let's look deeper and
find why it was added there.
The rtnl_lock() was added to asynchronous handlers by commit 338c15
("e1000: fix occasional panic on unload") in order to prevent
asynchronous handlers from execution after the module is unloaded
(e1000_down() is called) as it follows from the comment to the commit:
> 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.
I'm not sure if this locking scheme fixed the problem or just made it
unlikely, although I incline to the latter. Anyway, this was long time
ago when e1000 auxiliary works were implemented as timers scheduling
real work handlers in their routines. The e1000_down() function only
canceled the timers, but left the real handlers running if they were
running, which could result in work execution after module unload.
Today, the e1000 driver uses sane delayed works instead of the pair
timer+work to implement its delayed asynchronous handlers, and the
e1000_down() synchronously cancels all the works so that the problem
that commit 338c15 tried to cope with disappeared, and we don't need any
locks in the handlers any more. Moreover, any locking there can
potentially result in a deadlock.
So, this patch reverts commits 0ef4ee and 338c15.
Fixes: 0ef4eedc2e98 ("e1000: convert to private mutex from rtnl")
Fixes: 338c15e470d8 ("e1000: fix occasional panic on unload")
Cc: Tushar Dave <tushar.n.dave@intel.com>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000.h | 2 --
drivers/net/ethernet/intel/e1000/e1000_main.c | 36 +++------------------------
2 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
index e4093d1..f9313b3 100644
--- a/drivers/net/ethernet/intel/e1000/e1000.h
+++ b/drivers/net/ethernet/intel/e1000/e1000.h
@@ -317,8 +317,6 @@ struct e1000_adapter {
struct delayed_work watchdog_task;
struct delayed_work fifo_stall_task;
struct delayed_work phy_info_task;
-
- struct mutex mutex;
};
enum e1000_state_t {
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index c0f5217..619b0cb 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -544,21 +544,8 @@ void e1000_down(struct e1000_adapter *adapter)
e1000_clean_all_rx_rings(adapter);
}
-static void e1000_reinit_safe(struct e1000_adapter *adapter)
-{
- while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
- msleep(1);
- mutex_lock(&adapter->mutex);
- e1000_down(adapter);
- e1000_up(adapter);
- mutex_unlock(&adapter->mutex);
- 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);
@@ -1316,7 +1303,6 @@ static int e1000_sw_init(struct e1000_adapter *adapter)
e1000_irq_disable(adapter);
spin_lock_init(&adapter->stats_lock);
- mutex_init(&adapter->mutex);
set_bit(__E1000_DOWN, &adapter->flags);
@@ -2329,11 +2315,8 @@ static void e1000_update_phy_info_task(struct work_struct *work)
struct e1000_adapter *adapter = container_of(work,
struct e1000_adapter,
phy_info_task.work);
- if (test_bit(__E1000_DOWN, &adapter->flags))
- return;
- mutex_lock(&adapter->mutex);
+
e1000_phy_get_info(&adapter->hw, &adapter->phy_info);
- mutex_unlock(&adapter->mutex);
}
/**
@@ -2349,9 +2332,6 @@ static void e1000_82547_tx_fifo_stall_task(struct work_struct *work)
struct net_device *netdev = adapter->netdev;
u32 tctl;
- if (test_bit(__E1000_DOWN, &adapter->flags))
- return;
- mutex_lock(&adapter->mutex);
if (atomic_read(&adapter->tx_fifo_stall)) {
if ((er32(TDT) == er32(TDH)) &&
(er32(TDFT) == er32(TDFH)) &&
@@ -2372,7 +2352,6 @@ static void e1000_82547_tx_fifo_stall_task(struct work_struct *work)
schedule_delayed_work(&adapter->fifo_stall_task, 1);
}
}
- mutex_unlock(&adapter->mutex);
}
bool e1000_has_link(struct e1000_adapter *adapter)
@@ -2426,10 +2405,6 @@ static void e1000_watchdog(struct work_struct *work)
struct e1000_tx_ring *txdr = adapter->tx_ring;
u32 link, tctl;
- if (test_bit(__E1000_DOWN, &adapter->flags))
- return;
-
- mutex_lock(&adapter->mutex);
link = e1000_has_link(adapter);
if ((netif_carrier_ok(netdev)) && link)
goto link_up;
@@ -2520,7 +2495,7 @@ link_up:
adapter->tx_timeout_count++;
schedule_work(&adapter->reset_task);
/* exit immediately since reset is imminent */
- goto unlock;
+ return;
}
}
@@ -2548,9 +2523,6 @@ link_up:
/* Reschedule the task */
if (!test_bit(__E1000_DOWN, &adapter->flags))
schedule_delayed_work(&adapter->watchdog_task, 2 * HZ);
-
-unlock:
- mutex_unlock(&adapter->mutex);
}
enum latency_range {
@@ -3499,10 +3471,8 @@ static void e1000_reset_task(struct work_struct *work)
struct e1000_adapter *adapter =
container_of(work, struct e1000_adapter, reset_task);
- if (test_bit(__E1000_DOWN, &adapter->flags))
- return;
e_err(drv, "Reset adapter\n");
- e1000_reinit_safe(adapter);
+ e1000_reinit_locked(adapter);
}
/**
--
1.8.3.1
^ permalink raw reply related
* [net 8/8] ixgbe: Make ixgbe_identify_qsfp_module_generic static
From: Jeff Kirsher @ 2013-11-28 7:36 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1385624162-10116-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mark Rustad <mark.d.rustad@intel.com>
Correct a namespace complaint by making the function static
and moving the prototype into the .c file.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 3 ++-
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 1 -
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index e4c6760..39217e5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -46,6 +46,7 @@ static bool ixgbe_get_i2c_data(u32 *i2cctl);
static void ixgbe_i2c_bus_clear(struct ixgbe_hw *hw);
static enum ixgbe_phy_type ixgbe_get_phy_type_from_id(u32 phy_id);
static s32 ixgbe_get_phy_id(struct ixgbe_hw *hw);
+static s32 ixgbe_identify_qsfp_module_generic(struct ixgbe_hw *hw);
/**
* ixgbe_identify_phy_generic - Get physical layer module
@@ -1164,7 +1165,7 @@ err_read_i2c_eeprom:
*
* Searches for and identifies the QSFP module and assigns appropriate PHY type
**/
-s32 ixgbe_identify_qsfp_module_generic(struct ixgbe_hw *hw)
+static s32 ixgbe_identify_qsfp_module_generic(struct ixgbe_hw *hw)
{
struct ixgbe_adapter *adapter = hw->back;
s32 status = IXGBE_ERR_PHY_ADDR_INVALID;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
index aae900a..fffcbdd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
@@ -145,7 +145,6 @@ s32 ixgbe_get_phy_firmware_version_generic(struct ixgbe_hw *hw,
s32 ixgbe_reset_phy_nl(struct ixgbe_hw *hw);
s32 ixgbe_identify_module_generic(struct ixgbe_hw *hw);
s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw);
-s32 ixgbe_identify_qsfp_module_generic(struct ixgbe_hw *hw);
s32 ixgbe_get_sfp_init_sequence_offsets(struct ixgbe_hw *hw,
u16 *list_offset,
u16 *data_offset);
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH net] r8169: check ALDPS bit and disable it if enabledforthe 8168g
From: hayeswang @ 2013-11-28 7:58 UTC (permalink / raw)
To: 'David Chang'
Cc: netdev, 'David S. Miller', 'Francois Romieu',
jlee, 'nic_swsd'
In-Reply-To: <20131128062257.GB9297@linux-g7tt.site>
David Chang [mailto:dchang@suse.com]
> Sent: Thursday, November 28, 2013 2:23 PM
> To: Hayeswang
> Cc: netdev@vger.kernel.org; 'David S. Miller'; 'Francois
> Romieu'; jlee@suse.com; nic_swsd
> Subject: Re: [PATCH net] r8169: check ALDPS bit and disable
> it if enabledforthe 8168g
>
> On Thu, Nov 28, 2013 at 01:28:08PM +0800, hayeswang wrote:
> > David Chang [mailto:dchang@suse.com]
> > > Sent: Thursday, November 28, 2013 12:29 PM
> > > To: Hayeswang
> > > Cc: netdev@vger.kernel.org; 'David S. Miller'; 'Francois
> > > Romieu'; jlee@suse.com; nic_swsd
> > > Subject: Re: [PATCH net] r8169: check ALDPS bit and disable
> > > it if enabled forthe 8168g
> > [...]
> > > > You have to delay at least 10 ms after disable it.
> > > > This makes sure the hw completes the action.
> > > >
> > >
> > > Thanks for your comment.
> > > I'd add a delay time and send the v2 version.
> >
> > Excuse me. Our engineers have different opinions about the delay.
> > I would comfirm it first.
> >
> OK! I'd wait until the final result, thanks!
The delay is unnecessary. Thanks.
Acked-by: Hayes Wang <hayeswang@realtek.com>
^ permalink raw reply
* [Patch v3] IPv6: Fixed support for blackhole and prohibit routes
From: Kamala R @ 2013-11-28 9:12 UTC (permalink / raw)
To: Hannes Frederic Sowa, netdev, David Miller; +Cc: linux-kernel, Kamala R
From: Kamala R <kamala@aristanetworks.com>
The behaviour of blackhole and prohibit routes has been corrected by setting
the input and output pointers of the dst variable appropriately. For
blackhole routes, they are set to dst_discard and to ip6_pkt_discard and
ip6_pkt_discard_out respectively for prohibit routes.
Signed-off-by: Kamala R <kamala@aristanetworks.com>
---
net/ipv6/route.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7faa9d5..97125c4 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1565,21 +1565,24 @@ int ip6_route_add(struct fib6_config *cfg)
goto out;
}
}
- rt->dst.output = ip6_pkt_discard_out;
- rt->dst.input = ip6_pkt_discard;
rt->rt6i_flags = RTF_REJECT|RTF_NONEXTHOP;
switch (cfg->fc_type) {
case RTN_BLACKHOLE:
rt->dst.error = -EINVAL;
+ rt->dst.output = dst_discard;
+ rt->dst.input = dst_discard;
break;
case RTN_PROHIBIT:
rt->dst.error = -EACCES;
+ rt->dst.output = ip6_pkt_prohibit_out;
+ rt->dst.input = ip6_pkt_prohibit;
break;
case RTN_THROW:
- rt->dst.error = -EAGAIN;
- break;
default:
- rt->dst.error = -ENETUNREACH;
+ rt->dst.error = (cfg->fc_type == RTN_THROW) ? -EAGAIN
+ : -ENETUNREACH;
+ rt->dst.output = ip6_pkt_discard_out;
+ rt->dst.input = ip6_pkt_discard;
break;
}
goto install_route;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/4] pch_gbe fixes and enhancements
From: Ondrej Puzman @ 2013-11-28 9:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Ondrej Puzman
In-Reply-To: <20131127.203929.1379316258754767343.davem@davemloft.net>
Hi,
I'm sending the pch_gbe patch set again per David Miller's request.
Now the patches should be correctly numbered.
The patches fixes problems I found on our industrial computer with Intel Atom E680T CPU and EG20T chipset.
Ondrej Puzman (4):
pch_gbe: Fix transmit queue management
pch_gbe: Remove tx_queue_len setting from the driver
pch_gbe: Add DQL support to the driver
pch_gbe: Fix: Link configuration in module parameters was ignored
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 2 --
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 4 ----
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 18 +++++++++++++-----
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c | 13 +------------
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h | 1 -
5 files changed, 14 insertions(+), 24 deletions(-)
--
1.7.2.3
^ permalink raw reply
* [PATCH 1/4] pch_gbe: Fix transmit queue management
From: Ondrej Puzman @ 2013-11-28 9:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Ondrej Puzman
In-Reply-To: <20131127.203929.1379316258754767343.davem@davemloft.net>
According to Documentation/networking/driver.txt the ndo_start_xmit method should not return NETDEV_TX_BUSY under normal circumstances.
Stop the transmit queue when tx_ring is full.
Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 27ffe0e..6764e59 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2157,6 +2157,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
/* CRC,ITAG no support */
pch_gbe_tx_queue(adapter, tx_ring, skb);
+
+ if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
+ netif_stop_queue(netdev);
+
spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
return NETDEV_TX_OK;
}
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/4] pch_gbe: Remove tx_queue_len setting from the driver
From: Ondrej Puzman @ 2013-11-28 9:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Ondrej Puzman
In-Reply-To: <20131127.203929.1379316258754767343.davem@davemloft.net>
The driver should not touch netdev tx_queue_len when changing link speed.
Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 2 --
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 5 -----
2 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 2a900307..f450d09 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -615,7 +615,6 @@ struct pch_gbe_privdata {
* @tx_ring: Pointer of Tx descriptor ring structure
* @rx_ring: Pointer of Rx descriptor ring structure
* @rx_buffer_len: Receive buffer length
- * @tx_queue_len: Transmit queue length
* @have_msi: PCI MSI mode flag
* @pch_gbe_privdata: PCI Device ID driver_data
*/
@@ -639,7 +638,6 @@ struct pch_gbe_adapter {
struct pch_gbe_tx_ring *tx_ring;
struct pch_gbe_rx_ring *rx_ring;
unsigned long rx_buffer_len;
- unsigned long tx_queue_len;
bool have_msi;
bool rx_stop_flag;
int hwts_tx_en;
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 6764e59..d967603 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1076,11 +1076,9 @@ static void pch_gbe_set_mode(struct pch_gbe_adapter *adapter, u16 speed,
switch (speed) {
case SPEED_10:
mode = PCH_GBE_MODE_MII_ETHER;
- netdev->tx_queue_len = 10;
break;
case SPEED_100:
mode = PCH_GBE_MODE_MII_ETHER;
- netdev->tx_queue_len = 100;
break;
case SPEED_1000:
mode = PCH_GBE_MODE_GMII_ETHER;
@@ -1108,7 +1106,6 @@ static void pch_gbe_watchdog(unsigned long data)
pch_gbe_update_stats(adapter);
if ((mii_link_ok(&adapter->mii)) && (!netif_carrier_ok(netdev))) {
struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
- netdev->tx_queue_len = adapter->tx_queue_len;
/* mii library handles link maintenance tasks */
if (mii_ethtool_gset(&adapter->mii, &cmd)) {
netdev_err(netdev, "ethtool get setting Error\n");
@@ -1975,7 +1972,6 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
}
pch_gbe_alloc_tx_buffers(adapter, tx_ring);
pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
- adapter->tx_queue_len = netdev->tx_queue_len;
pch_gbe_enable_dma_rx(&adapter->hw);
pch_gbe_enable_mac_rx(&adapter->hw);
@@ -2013,7 +2009,6 @@ void pch_gbe_down(struct pch_gbe_adapter *adapter)
del_timer_sync(&adapter->watchdog_timer);
- netdev->tx_queue_len = adapter->tx_queue_len;
netif_carrier_off(netdev);
netif_stop_queue(netdev);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/4] pch_gbe: Fix: Link configuration in module parameters was ignored
From: Ondrej Puzman @ 2013-11-28 9:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Ondrej Puzman
In-Reply-To: <20131127.203929.1379316258754767343.davem@davemloft.net>
PHY configuration set in pch_gbe_phy_init_setting() by mii_ethtool_sset() was immedeiately reset by two calls of pch_gbe_phy_sw_reset().
Result of this bug was that module parameters like AutoNeg were ignored silently.
Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 4 ----
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c | 13 +------------
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h | 1 -
3 files changed, 1 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index ff3ad70..365c202 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -77,10 +77,6 @@ static s32 pch_gbe_plat_init_hw(struct pch_gbe_hw *hw)
return ret_val;
}
pch_gbe_phy_init_setting(hw);
- /* Setup Mac interface option RGMII */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
- pch_gbe_phy_set_rgmii(hw);
-#endif
return ret_val;
}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index 8b7ff75..d283818 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -249,15 +249,6 @@ void pch_gbe_phy_power_down(struct pch_gbe_hw *hw)
}
/**
- * pch_gbe_phy_set_rgmii - RGMII interface setting
- * @hw: Pointer to the HW structure
- */
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw)
-{
- pch_gbe_phy_sw_reset(hw);
-}
-
-/**
* pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
* @hw: Pointer to the HW structure
* Returns
@@ -321,13 +312,11 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
cmd.duplex = hw->mac.link_duplex;
cmd.advertising = hw->phy.autoneg_advertised;
cmd.autoneg = hw->mac.autoneg;
- pch_gbe_phy_write_reg_miic(hw, MII_BMCR, BMCR_RESET);
+ pch_gbe_phy_sw_reset(hw);
ret = mii_ethtool_sset(&adapter->mii, &cmd);
if (ret)
netdev_err(adapter->netdev, "Error: mii_ethtool_sset\n");
- pch_gbe_phy_sw_reset(hw);
-
pch_gbe_phy_read_reg_miic(hw, PHY_PHYSP_CONTROL, &mii_reg);
mii_reg |= PHYSP_CTRL_ASSERT_CRS_TX;
pch_gbe_phy_write_reg_miic(hw, PHY_PHYSP_CONTROL, mii_reg);
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index 0cbe692..cda223c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -31,7 +31,6 @@ void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw);
void pch_gbe_phy_hw_reset(struct pch_gbe_hw *hw);
void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/4] pch_gbe: Add DQL support to the driver
From: Ondrej Puzman @ 2013-11-28 9:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Ondrej Puzman
In-Reply-To: <20131127.203929.1379316258754767343.davem@davemloft.net>
Add dynamic queue limits support to the driver.
Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index d967603..2c14e2e 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -994,6 +994,8 @@ static void pch_gbe_clean_tx_ring(struct pch_gbe_adapter *adapter,
netdev_dbg(adapter->netdev,
"call pch_gbe_unmap_and_free_tx_resource() %d count\n", i);
+ netdev_reset_queue(adapter->netdev);
+
size = (unsigned long)sizeof(struct pch_gbe_buffer) * tx_ring->count;
memset(tx_ring->buffer_info, 0, size);
@@ -1544,6 +1546,7 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
unsigned int cleaned_count = 0;
bool cleaned = false;
int unused, thresh;
+ unsigned int bytes_compl = 0, pkts_compl = 0;
netdev_dbg(adapter->netdev, "next_to_clean : %d\n",
tx_ring->next_to_clean);
@@ -1622,6 +1625,8 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
buffer_info->mapped = false;
}
if (buffer_info->skb) {
+ bytes_compl += buffer_info->skb->len;
+ pkts_compl++;
netdev_dbg(adapter->netdev,
"trim buffer_info->skb : %d\n", i);
skb_trim(buffer_info->skb, 0);
@@ -1641,6 +1646,9 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
"called pch_gbe_unmap_and_free_tx_resource() %d count\n",
cleaned_count);
if (cleaned_count > 0) { /*skip this if nothing cleaned*/
+ netdev_completed_queue(adapter->netdev,
+ pkts_compl, bytes_compl);
+
/* Recover from running out of Tx resources in xmit_frame */
spin_lock(&tx_ring->tx_lock);
if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev))))
@@ -2152,6 +2160,7 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
/* CRC,ITAG no support */
pch_gbe_tx_queue(adapter, tx_ring, skb);
+ netdev_sent_queue(netdev, skb->len);
if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
netif_stop_queue(netdev);
--
1.7.2.3
^ permalink raw reply related
* packet: use macro GET_PBDQC_FROM_RB to simplify the codes
From: Duan Jiong @ 2013-11-28 9:53 UTC (permalink / raw)
To: David Miller, dborkman; +Cc: netdev
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
net/packet/af_packet.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..955611f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -430,7 +430,8 @@ static void prb_shutdown_retire_blk_timer(struct packet_sock *po,
{
struct tpacket_kbdq_core *pkc;
- pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
+ pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring)
+ : GET_PBDQC_FROM_RB(&po->rx_ring);
spin_lock(&rb_queue->lock);
pkc->delete_blk_timer = 1;
@@ -456,7 +457,8 @@ static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
if (tx_ring)
BUG();
- pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
+ pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring)
+ : GET_RBDQC_FROM_RB(&po->rx_ring);
prb_init_blk_timer(po, pkc, prb_retire_rx_blk_timer_expired);
}
@@ -514,7 +516,7 @@ static void init_prb_bdqc(struct packet_sock *po,
struct pgv *pg_vec,
union tpacket_req_u *req_u, int tx_ring)
{
- struct tpacket_kbdq_core *p1 = &rb->prb_bdqc;
+ struct tpacket_kbdq_core *p1 = GET_PBDQC_FROM_RB(rb);
struct tpacket_block_desc *pbd;
memset(p1, 0x0, sizeof(*p1));
@@ -578,7 +580,7 @@ static void _prb_refresh_rx_retire_blk_timer(struct tpacket_kbdq_core *pkc)
static void prb_retire_rx_blk_timer_expired(unsigned long data)
{
struct packet_sock *po = (struct packet_sock *)data;
- struct tpacket_kbdq_core *pkc = &po->rx_ring.prb_bdqc;
+ struct tpacket_kbdq_core *pkc = GET_PBDQC_FROM_RB(&po->rx_ring);
unsigned int frozen;
struct tpacket_block_desc *pbd;
--
1.8.3.1
^ permalink raw reply related
* xs
From: nuriyanto @ 2013-11-28 10:08 UTC (permalink / raw)
Re: Nokia has picked you as a lucky person for a lump sum give out of £2,500,000.00, send your, Names,Add Names,Phone No
^ permalink raw reply
* Re: packet: use macro GET_PBDQC_FROM_RB to simplify the codes
From: Daniel Borkmann @ 2013-11-28 10:27 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, netdev
In-Reply-To: <529712A7.3010604@cn.fujitsu.com>
On 11/28/2013 10:53 AM, Duan Jiong wrote:
>
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Please resubmit when net-next is open.
> ---
> net/packet/af_packet.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 2e8286b..955611f 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -430,7 +430,8 @@ static void prb_shutdown_retire_blk_timer(struct packet_sock *po,
> {
> struct tpacket_kbdq_core *pkc;
>
> - pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
> + pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring)
> + : GET_PBDQC_FROM_RB(&po->rx_ring);
Nit: the colon should actually still be on the previous line.
It would be nice if at some point in time we could get rid of these TPACKET_V3 specific
macros and actually use some inline functions.
>
> spin_lock(&rb_queue->lock);
> pkc->delete_blk_timer = 1;
> @@ -456,7 +457,8 @@ static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
> if (tx_ring)
> BUG();
>
> - pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
> + pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring)
> + : GET_RBDQC_FROM_RB(&po->rx_ring);
> prb_init_blk_timer(po, pkc, prb_retire_rx_blk_timer_expired);
> }
>
> @@ -514,7 +516,7 @@ static void init_prb_bdqc(struct packet_sock *po,
> struct pgv *pg_vec,
> union tpacket_req_u *req_u, int tx_ring)
> {
> - struct tpacket_kbdq_core *p1 = &rb->prb_bdqc;
> + struct tpacket_kbdq_core *p1 = GET_PBDQC_FROM_RB(rb);
> struct tpacket_block_desc *pbd;
>
> memset(p1, 0x0, sizeof(*p1));
> @@ -578,7 +580,7 @@ static void _prb_refresh_rx_retire_blk_timer(struct tpacket_kbdq_core *pkc)
> static void prb_retire_rx_blk_timer_expired(unsigned long data)
> {
> struct packet_sock *po = (struct packet_sock *)data;
> - struct tpacket_kbdq_core *pkc = &po->rx_ring.prb_bdqc;
> + struct tpacket_kbdq_core *pkc = GET_PBDQC_FROM_RB(&po->rx_ring);
> unsigned int frozen;
> struct tpacket_block_desc *pbd;
>
>
^ permalink raw reply
* Re: [PATCH 1/2] virtio_net: fix error handling for mergeable buffers
From: Michael S. Tsirkin @ 2013-11-28 11:14 UTC (permalink / raw)
To: Jason Wang
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <5296B525.5060509@redhat.com>
On Thu, Nov 28, 2013 at 11:14:45AM +0800, Jason Wang wrote:
> On 11/28/2013 12:31 AM, Michael S. Tsirkin wrote:
> > Eric Dumazet noticed that if we encounter an error
> > when processing a mergeable buffer, we don't
> > dequeue all of the buffers from this packet,
> > the result is almost sure to be loss of networking.
> >
> > Jason Wang noticed that we also leak a page and that we don't decrement
> > the rq buf count, so we won't repost buffers (a resource leak).
> >
> > Fix both issues, and also make the logic a bit more
> > robust against device errors by not looping when e.g. because of a leak
> > like the one we are fixing here the number of buffers is 0.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: Michael Dalton <mwdalton@google.com>
> > Reported-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Note: this bugfix is needed on stable too, but backport
> > might not be trivial.
> > I'll send a backport for stable separately.
>
> That will be fine.
> >
> > drivers/net/virtio_net.c | 84 ++++++++++++++++++++++++++++++------------------
> > 1 file changed, 52 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 7bab4de..0e6ea69 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -299,41 +299,53 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> > return skb;
> > }
> >
> > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> > +static struct sk_buff *receive_mergeable(struct net_device *dev,
> > + struct receive_queue *rq,
> > + void *buf,
> > + unsigned int len)
> > {
> > - struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
> > + struct skb_vnet_hdr *hdr = buf;
> > + int num_buf = hdr->mhdr.num_buffers;
> > + struct page *page = virt_to_head_page(buf);
> > + int offset = buf - page_address(page);
> > + struct sk_buff *head_skb = page_to_skb(rq, page, offset, len,
> > + MERGE_BUFFER_LEN);
> > struct sk_buff *curr_skb = head_skb;
> > - char *buf;
> > - struct page *page;
> > - int num_buf, len, offset;
> >
> > - num_buf = hdr->mhdr.num_buffers;
> > + if (unlikely(!curr_skb))
> > + goto err_skb;
> > +
> > while (--num_buf) {
> > - int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> > + int num_skb_frags;
> > +
> > buf = virtqueue_get_buf(rq->vq, &len);
> > if (unlikely(!buf)) {
> > - pr_debug("%s: rx error: %d buffers missing\n",
> > - head_skb->dev->name, hdr->mhdr.num_buffers);
> > - head_skb->dev->stats.rx_length_errors++;
> > - return -EINVAL;
> > + pr_debug("%s: rx error: %d buffers out of %d missing\n",
> > + dev->name, num_buf, hdr->mhdr.num_buffers);
> > + dev->stats.rx_length_errors++;
> > + goto err_buf;
>
> Not sure it's correct here. Since the we break immediately if buffer is
> missed in err_buf and rx_length_error will be miss counted.
Note that this never happens: it's an internal device error, we are
just trying to be defensive here. I would not worry about getting
error counters right in this case.
But I agree - since we know virtqueue_get_buf failed it's useless to try
and get more. So let's move the loop before the err_buf label.
> Maybe an ERR_PTR(-EINVAL) is better.
This will just make callers worry about checking IS_ERR, and
caller returns void so it has nothing to do with the return code anyway.
> > }
> > if (unlikely(len > MERGE_BUFFER_LEN)) {
> > pr_debug("%s: rx error: merge buffer too long\n",
> > - head_skb->dev->name);
> > + dev->name);
> > len = MERGE_BUFFER_LEN;
> > }
> > +
> > + page = virt_to_head_page(buf);
> > + --rq->num;
> > +
> > + num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> > if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> > struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> > - if (unlikely(!nskb)) {
> > - head_skb->dev->stats.rx_dropped++;
> > - return -ENOMEM;
> > - }
> > +
> > + if (unlikely(!nskb))
> > + goto err_skb;
> > if (curr_skb == head_skb)
> > skb_shinfo(curr_skb)->frag_list = nskb;
> > else
> > curr_skb->next = nskb;
> > - curr_skb = nskb;
> > head_skb->truesize += nskb->truesize;
> > + curr_skb = nskb;
>
> This change seems unnecessary.
Right, I'll drop it.
> Other looks good.
> > num_skb_frags = 0;
> > }
> > if (curr_skb != head_skb) {
> > @@ -341,8 +353,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> > head_skb->len += len;
> > head_skb->truesize += MERGE_BUFFER_LEN;
> > }
> > - page = virt_to_head_page(buf);
> > - offset = buf - (char *)page_address(page);
> > + offset = buf - page_address(page);
> > if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
> > put_page(page);
> > skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
> > @@ -351,9 +362,28 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> > skb_add_rx_frag(curr_skb, num_skb_frags, page,
> > offset, len, MERGE_BUFFER_LEN);
> > }
> > + }
> > +
> > + return head_skb;
> > +
> > +err_skb:
> > + put_page(page);
> > +err_buf:
> > + dev->stats.rx_dropped++;
> > + dev_kfree_skb(head_skb);
> > + while (--num_buf) {
> > + buf = virtqueue_get_buf(rq->vq, &len);
> > + if (unlikely(!buf)) {
> > + pr_debug("%s: rx error: %d buffers missing\n",
> > + dev->name, num_buf);
> > + dev->stats.rx_length_errors++;
> > + break;
> > + }
> > + page = virt_to_head_page(buf);
> > + put_page(page);
> > --rq->num;
> > }
> > - return 0;
> > + return NULL;
> > }
> >
> > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > @@ -382,19 +412,9 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > len -= sizeof(struct virtio_net_hdr);
> > skb_trim(skb, len);
> > } else if (vi->mergeable_rx_bufs) {
> > - struct page *page = virt_to_head_page(buf);
> > - skb = page_to_skb(rq, page,
> > - (char *)buf - (char *)page_address(page),
> > - len, MERGE_BUFFER_LEN);
> > - if (unlikely(!skb)) {
> > - dev->stats.rx_dropped++;
> > - put_page(page);
> > + skb = receive_mergeable(dev, rq, buf, len);
> > + if (unlikely(!skb))
> > return;
> > - }
> > - if (receive_mergeable(rq, skb)) {
> > - dev_kfree_skb(skb);
> > - return;
> > - }
> > } else {
> > page = buf;
> > skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
^ permalink raw reply
* Re: [PATCH] net: fec_main: dma_map() only the length of the skb
From: Sebastian Andrzej Siewior @ 2013-11-28 11:24 UTC (permalink / raw)
To: Fugang Duan
Cc: netdev@vger.kernel.org, David S. Miller, Fabio Estevam,
Jim Baxter, Marek Szyprowski
In-Reply-To: <9848F2DB572E5649BA045B288BE08FBE0189C105@039-SN2MPN1-023.039d.mgd.msft.net>
On 11/28/2013 02:18 AM, Fugang Duan wrote:
- <frank.li@freescale.net> because MTA complains (now really)
>> On 11/27/2013 02:08 PM, Fugang Duan wrote:
>>> In fact, there have one memory copy for enet as below since enet have 16
>> bytes data buffer alignment request.
>>> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
>>> memcpy(fep->tx_bounce[index], skb->data, skb->len);
>>> bufaddr = fep->tx_bounce[index]; }
>>
>> This memcpy() is only executed if the buffer isn't properly aligned which
>> shouldn't be the rule but an exception. This aligment check is also available
>> in v2.6.31 where the BUG_ON() statement was triggered.
>>
> In fact, the skb->data address is not aligned with FEC_ALIGNMENT, so memcpy() always is called at here.
I am aware of NET_IP_ALIGN. The pointers I mentioned in the log were
from a printk() shortly before that bug triggered. So I am not making
this up :) I am not aware of the load that triggered this.
Anyway, adding this
WARN_ON(!((unsigned long)skb->data & 0xf));
in my e1000 on v3.12 triggers shortly after boot and according to the
backtrace it comes aoe. So that is at least one user :)
Since that aligment is for IP, I am not sure if non-IP based protocol
are using this. I know however that the workload, that triggered the
problem, is not IP-based.
>
> Thanks,
> Andy
Sebastian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox