* [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference
From: Nikolay Aleksandrov @ 2013-04-11 13:45 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1365538644-1502-1-git-send-email-nikolay@redhat.com>
In commit 471cb5a33dcbd7c529684a2ac7ba4451414ee4a7 ("bonding: remove
usage of dev->master") a bug was introduced which causes a NULL pointer
dereference. If a bond device is in mode 6 (ALB) and a slave is added
it will dereference a NULL pointer in bond_slave_netdev_event().
This is because in bond_enslave we have bond_alb_init_slave() which
changes the MAC address of the slave and causes a NETDEV_CHANGEADDR.
Then we have in bond_slave_netdev_event():
struct slave *slave = bond_slave_get_rtnl(slave_dev);
struct bonding *bond = slave->bond;
bond_slave_get_rtnl() dereferences slave_dev->rx_handler_data which at
that time is NULL since netdev_rx_handler_register() is called later.
This is fixed by checking if slave is NULL before dereferencing it.
v2: Fix the comment styling
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 171b10f..15c675c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3168,11 +3168,20 @@ static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev)
{
struct slave *slave = bond_slave_get_rtnl(slave_dev);
- struct bonding *bond = slave->bond;
- struct net_device *bond_dev = slave->bond->dev;
+ struct bonding *bond;
+ struct net_device *bond_dev;
u32 old_speed;
u8 old_duplex;
+ /* A netdev event can be generated while enslaving a device
+ * before netdev_rx_handler_register is called in which case
+ * slave will be NULL
+ */
+ if (!slave)
+ return NOTIFY_DONE;
+ bond_dev = slave->bond->dev;
+ bond = slave->bond;
+
switch (event) {
case NETDEV_UNREGISTER:
if (bond->setup_by_slave)
--
1.8.1.4
^ permalink raw reply related
* Re: Netpoll triggers soft lockup
From: Neil Horman @ 2013-04-11 14:08 UTC (permalink / raw)
To: Bart Van Assche; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <5166BDAA.3000603@acm.org>
On Thu, Apr 11, 2013 at 03:42:02PM +0200, Bart Van Assche wrote:
> Hi,
>
> While testing a driver against kernel 3.9-rc6 I ran into a soft
> lockup triggered by sending lots of kernel messages to a remote
> system via netconsole. This behavior was probably introduced by
> commit ca99ca14c ("netpoll: protect napi_poll and poll_controller
> during dev_[open|close]"). That commit introduced a mutex in
> netpoll_poll_dev(), which can be called from interrupt context. Is
> there anyone who can tell me whether this is a bug in commit
> ca99ca14c or in netconsole ?
>
Thanks for the report Bart, thats interesting. You're right, the problem was
introduced by ca99ca14c, but it seems like a false positive to me. The warning
is comming from DEBUG_LOCKS_WARN_ON in spin_lock_mutex. It appears that the
intent here is to catch users of spin_lock_mutex (which is the mutex api), from
trying to use mutex locking in interrupt context as that results in a panic. In
this case however, we're using the mutex_trylock call, which is guaranteed not
to sleep (which is the nature of the panic condition). So we really don't need
to warn here. I think the right thing to do in this case is to add a flag to
spin_lock_mutex that skips the warning if we're doing a trylock operation. I'll
work up a patch shortly.
Thanks again!
Neil
^ permalink raw reply
* [net-next PATCH] be2net: remove unused variable 'sge'
From: Ivan Vecera @ 2013-04-11 14:29 UTC (permalink / raw)
To: netdev; +Cc: sathya.perla, subbu.seetharaman, ajit.khaparde
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index f286ad2..cf9408f 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -2343,7 +2343,6 @@ int be_cmd_get_seeprom_data(struct be_adapter *adapter,
{
struct be_mcc_wrb *wrb;
struct be_cmd_req_seeprom_read *req;
- struct be_sge *sge;
int status;
spin_lock_bh(&adapter->mcc_lock);
@@ -2354,7 +2353,6 @@ int be_cmd_get_seeprom_data(struct be_adapter *adapter,
goto err;
}
req = nonemb_cmd->va;
- sge = nonembedded_sgl(wrb);
be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
OPCODE_COMMON_SEEPROM_READ, sizeof(*req), wrb,
--
1.8.1.5
^ permalink raw reply related
* [PATCH 09/11] usbnet: sierra: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Use usbnet_link_change to handle link change centrally.
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/sierra_net.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 79ab243..a923d61 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -413,11 +413,10 @@ static void sierra_net_handle_lsi(struct usbnet *dev, char *data,
if (link_up) {
sierra_net_set_ctx_index(priv, hh->msgspecific.byte);
priv->link_up = 1;
- netif_carrier_on(dev->net);
} else {
priv->link_up = 0;
- netif_carrier_off(dev->net);
}
+ usbnet_link_change(dev, link_up, 0);
}
static void sierra_net_dosync(struct usbnet *dev)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 00/11] usbnet: usbnet: handle link change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman; +Cc: Oliver Neukum, netdev, linux-usb
Hi,
This patch set introduces usbnet_link_change() API and applies
it on all usbnet drivers, then handle the link change centrally
to stop bulk transfer when link becomes off and restart bulk
transfer when link becomes on.
With the change, ~10% performance boost on bulk transfer
of another device on the same bus can be obtained when link
is off. Also, stopping bulk transfer when link becomes off
may disable asynchonous schedule of host controller, power
might be saved probabally.
drivers/net/usb/asix_devices.c | 6 +-----
drivers/net/usb/ax88179_178a.c | 12 ++++-------
drivers/net/usb/cdc_ether.c | 5 +----
drivers/net/usb/cdc_ncm.c | 9 +++-----
drivers/net/usb/dm9601.c | 7 +------
drivers/net/usb/mcs7830.c | 6 +-----
drivers/net/usb/sierra_net.c | 3 +--
drivers/net/usb/usbnet.c | 45 +++++++++++++++++++++++++++++++++++++++-
include/linux/usb/usbnet.h | 2 ++
9 files changed, 58 insertions(+), 37 deletions(-)
Thanks,
--
Ming Lei
^ permalink raw reply
* [PATCH 01/11] usbnet: introduce usbnet_link_change API
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
This patch introduces the API of usbnet_link_change, so that
usbnet can handle link change centrally, which may help to
implement killing traffic URBs for saving USB bus bandwidth
and host controller power.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/usbnet.c | 13 +++++++++++++
include/linux/usb/usbnet.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 51f3192..40e4237 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1653,6 +1653,19 @@ int usbnet_manage_power(struct usbnet *dev, int on)
}
EXPORT_SYMBOL(usbnet_manage_power);
+void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset)
+{
+ /* update link after link is reseted */
+ if (link && !need_reset)
+ netif_carrier_on(dev->net);
+ else
+ netif_carrier_off(dev->net);
+
+ if (need_reset && link)
+ usbnet_defer_kevent(dev, EVENT_LINK_RESET);
+}
+EXPORT_SYMBOL(usbnet_link_change);
+
/*-------------------------------------------------------------------------*/
static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, void *data, u16 size)
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 0e5ac93..eb021b8 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -245,5 +245,6 @@ extern void usbnet_get_drvinfo(struct net_device *, struct ethtool_drvinfo *);
extern int usbnet_nway_reset(struct net_device *net);
extern int usbnet_manage_power(struct usbnet *, int);
+extern void usbnet_link_change(struct usbnet *, bool, bool);
#endif /* __LINUX_USB_USBNET_H */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 02/11] usbnet: mcs7830: don't reset link
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
The driver doesn't implement link_reset() callback, so it needn't
to send link reset event.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/mcs7830.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index 3f3f566..e1c00e9 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -576,10 +576,9 @@ static void mcs7830_status(struct usbnet *dev, struct urb *urb)
*/
if (data->link_counter > 20) {
data->link_counter = 0;
- if (link) {
+ if (link)
netif_carrier_on(dev->net);
- usbnet_defer_kevent(dev, EVENT_LINK_RESET);
- } else
+ else
netif_carrier_off(dev->net);
netdev_dbg(dev->net, "Link Status is: %d\n", link);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 03/11] usbnet: mcs7830: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
This patch uses the introduced usbnet_link_change() to handle
link change.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/mcs7830.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index e1c00e9..03832d3 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -576,10 +576,7 @@ static void mcs7830_status(struct usbnet *dev, struct urb *urb)
*/
if (data->link_counter > 20) {
data->link_counter = 0;
- if (link)
- netif_carrier_on(dev->net);
- else
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, link, 0);
netdev_dbg(dev->net, "Link Status is: %d\n", link);
}
} else
--
1.7.9.5
^ permalink raw reply related
* [PATCH 04/11] usbnet: cdc_ncm: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use the introduced usbnet_link_change to handle link change.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/cdc_ncm.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 67012cb..43afde8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -610,7 +610,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
* (carrier is OFF) during attach, so the IP network stack does not
* start IPv6 negotiation and more.
*/
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, 0, 0);
return ret;
}
@@ -1106,12 +1106,9 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
" %sconnected\n",
ctx->netdev->name, ctx->connected ? "" : "dis");
- if (ctx->connected)
- netif_carrier_on(dev->net);
- else {
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, ctx->connected, 0);
+ if (!ctx->connected)
ctx->tx_speed = ctx->rx_speed = 0;
- }
break;
case USB_CDC_NOTIFY_SPEED_CHANGE:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 05/11] usbnet: asix: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use usbnet_link_change to handle link change centrally.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/asix_devices.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 7097534..ad5d1e4 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -55,11 +55,7 @@ static void asix_status(struct usbnet *dev, struct urb *urb)
event = urb->transfer_buffer;
link = event->link & 0x01;
if (netif_carrier_ok(dev->net) != link) {
- if (link) {
- netif_carrier_on(dev->net);
- usbnet_defer_kevent (dev, EVENT_LINK_RESET );
- } else
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, link, 1);
netdev_dbg(dev->net, "Link Status is: %d\n", link);
}
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 06/11] usbnet: ax88179_1781: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use usbnet_link_change to handle link change centrally.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/ax88179_178a.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 71c27d8..bd8758f 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -352,11 +352,7 @@ static void ax88179_status(struct usbnet *dev, struct urb *urb)
link = (((__force u32)event->intdata1) & AX_INT_PPLS_LINK) >> 16;
if (netif_carrier_ok(dev->net) != link) {
- if (link)
- usbnet_defer_kevent(dev, EVENT_LINK_RESET);
- else
- netif_carrier_off(dev->net);
-
+ usbnet_link_change(dev, link, 1);
netdev_info(dev->net, "ax88179 - Link status is: %d\n", link);
}
}
@@ -455,7 +451,7 @@ static int ax88179_resume(struct usb_interface *intf)
u16 tmp16;
u8 tmp8;
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, 0, 0);
/* Power up ethernet PHY */
tmp16 = 0;
@@ -1068,7 +1064,7 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
/* Restart autoneg */
mii_nway_restart(&dev->mii);
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, 0, 0);
return 0;
}
@@ -1356,7 +1352,7 @@ static int ax88179_reset(struct usbnet *dev)
/* Restart autoneg */
mii_nway_restart(&dev->mii);
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, 0, 0);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 07/11] usbnet: cdc-ether: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use usbnet_link_change to handle link change centrally.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/cdc_ether.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 57136dc..e965806 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -406,10 +406,7 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
case USB_CDC_NOTIFY_NETWORK_CONNECTION:
netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
event->wValue ? "on" : "off");
- if (event->wValue)
- netif_carrier_on(dev->net);
- else
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, event->wValue, 0);
break;
case USB_CDC_NOTIFY_SPEED_CHANGE: /* tx/rx rates */
netif_dbg(dev, timer, dev->net, "CDC: speed change (len %d)\n",
--
1.7.9.5
^ permalink raw reply related
* [PATCH 08/11] usbnet: dm9601: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei, Peter Korsgaard
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use usbnet_link_change to handle link change centrally.
Cc: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/dm9601.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index 174e5ec..2dbb946 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -524,12 +524,7 @@ static void dm9601_status(struct usbnet *dev, struct urb *urb)
link = !!(buf[0] & 0x40);
if (netif_carrier_ok(dev->net) != link) {
- if (link) {
- netif_carrier_on(dev->net);
- usbnet_defer_kevent (dev, EVENT_LINK_RESET);
- }
- else
- netif_carrier_off(dev->net);
+ usbnet_link_change(dev, link, 1);
netdev_dbg(dev->net, "Link Status is: %d\n", link);
}
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 10/11] usbnet: apply usbnet_link_change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
Use usbnet_link_change to handle link change centrally.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/usbnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 40e4237..34e4252 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1521,7 +1521,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
netif_device_attach (net);
if (dev->driver_info->flags & FLAG_LINK_INTR)
- netif_carrier_off(net);
+ usbnet_link_change(dev, 0, 0);
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 11/11] usbnet: handle link change
From: Ming Lei @ 2013-04-11 14:40 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
The link change is detected via the interrupt pipe, and bulk
pipes are responsible for transfering packets, so it is reasonable
to stop bulk transfer after link is reported as off.
Two adavantages may be obtained with stopping bulk transfer
after link becomes off:
- USB bus bandwidth is saved(USB bus is shared bus except for
USB3.0), for example, lots of 'IN' token packets and 'NYET'
handshake packets is transfered on 2.0 bus.
- probabaly power might be saved for usb host controller since
cancelling bulk transfer may disable the asynchronous schedule of
host controller.
With this patch, when link becomes off, about ~10% performance
boost can be found on bulk transfer of anther usb device which
is attached to same bus with the usbnet device, see below
test on next-20130410:
- read from usb mass storage(Sandisk Extreme USB 3.0) on pandaboard
with below command after unplugging ethernet cable:
dd if=/dev/sda iflag=direct of=/dev/null bs=1M count=500
- without the patch
1, 838860800 bytes (839 MB) copied, 36.2216 s, 23.2 MB/s
2, 838860800 bytes (839 MB) copied, 35.8368 s, 23.4 MB/s
3, 838860800 bytes (839 MB) copied, 35.823 s, 23.4 MB/s
4, 838860800 bytes (839 MB) copied, 35.937 s, 23.3 MB/s
5, 838860800 bytes (839 MB) copied, 35.7365 s, 23.5 MB/s
average: 23.6MB/s
- with the patch
1, 838860800 bytes (839 MB) copied, 32.3817 s, 25.9 MB/s
2, 838860800 bytes (839 MB) copied, 31.7389 s, 26.4 MB/s
3, 838860800 bytes (839 MB) copied, 32.438 s, 25.9 MB/s
4, 838860800 bytes (839 MB) copied, 32.5492 s, 25.8 MB/s
5, 838860800 bytes (839 MB) copied, 31.6178 s, 26.5 MB/s
average: 26.1MB/s
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/usbnet.c | 30 ++++++++++++++++++++++++++++++
include/linux/usb/usbnet.h | 1 +
2 files changed, 31 insertions(+)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 34e4252..1e5a9b7 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -938,6 +938,27 @@ static const struct ethtool_ops usbnet_ethtool_ops = {
/*-------------------------------------------------------------------------*/
+static void __handle_link_change(struct usbnet *dev)
+{
+ if (!test_bit(EVENT_DEV_OPEN, &dev->flags))
+ return;
+
+ if (!netif_carrier_ok(dev->net)) {
+ /* kill URBs for reading packets to save bus bandwidth */
+ unlink_urbs(dev, &dev->rxq);
+
+ /*
+ * tx_timeout will unlink URBs for sending packets and
+ * tx queue is stopped by netcore after link becomes off
+ */
+ } else {
+ /* submitting URBs for reading packets */
+ tasklet_schedule(&dev->bh);
+ }
+
+ clear_bit(EVENT_LINK_CHANGE, &dev->flags);
+}
+
/* work that cannot be done in interrupt context uses keventd.
*
* NOTE: with 2.5 we could do more of this using completion callbacks,
@@ -1035,8 +1056,14 @@ skip_reset:
} else {
usb_autopm_put_interface(dev->intf);
}
+
+ /* handle link change from link resetting */
+ __handle_link_change(dev);
}
+ if (test_bit (EVENT_LINK_CHANGE, &dev->flags))
+ __handle_link_change(dev);
+
if (dev->flags)
netdev_dbg(dev->net, "kevent done, flags = 0x%lx\n", dev->flags);
}
@@ -1286,6 +1313,7 @@ static void usbnet_bh (unsigned long param)
// or are we maybe short a few urbs?
} else if (netif_running (dev->net) &&
netif_device_present (dev->net) &&
+ netif_carrier_ok(dev->net) &&
!timer_pending (&dev->delay) &&
!test_bit (EVENT_RX_HALT, &dev->flags)) {
int temp = dev->rxq.qlen;
@@ -1663,6 +1691,8 @@ void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset)
if (need_reset && link)
usbnet_defer_kevent(dev, EVENT_LINK_RESET);
+ else
+ usbnet_defer_kevent(dev, EVENT_LINK_CHANGE);
}
EXPORT_SYMBOL(usbnet_link_change);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index eb021b8..da46327 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -72,6 +72,7 @@ struct usbnet {
# define EVENT_DEVICE_REPORT_IDLE 8
# define EVENT_NO_RUNTIME_PM 9
# define EVENT_RX_KILL 10
+# define EVENT_LINK_CHANGE 11
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] net: mv643xx_eth: Add GRO support
From: Sebastian Hesselbarth @ 2013-04-11 14:47 UTC (permalink / raw)
To: Willy Tarreau
Cc: Andrew Lunn, Jason Cooper, Benjamin Herrenschmidt, linux-kernel,
David S. Miller, Soeren Moch, Paul Mackerras, linux-arm-kernel,
Dale Farnsworth, netdev, linuxppc-dev, Florian Fainelli,
Lennert Buytenhek
In-Reply-To: <20130411131333.GD1910@1wt.eu>
On Thu, Apr 11, 2013 at 3:13 PM, Willy Tarreau <w@1wt.eu> wrote:
> On Thu, Apr 11, 2013 at 02:40:23PM +0200, Sebastian Hesselbarth wrote:
>> This patch adds GRO support to mv643xx_eth by making it invoke
>> napi_gro_receive instead of netif_receive_skb.
>>
>> Signed-off-by: Soeren Moch <smoch@web.de>
>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> ---
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Lennert Buytenhek <buytenh@wantstofly.org>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Jason Cooper <jason@lakedaemon.net>
>> Cc: Florian Fainelli <florian@openwrt.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Dale Farnsworth <dale@farnsworth.org>
>> Cc: netdev@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
>> index 305038f..c850d04 100644
>> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
>> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
>> @@ -604,7 +604,7 @@ static int rxq_process(struct rx_queue *rxq, int budget)
>> lro_receive_skb(&rxq->lro_mgr, skb, (void *)cmd_sts);
>> lro_flush_needed = 1;
>> } else
>> - netif_receive_skb(skb);
>> + napi_gro_receive(&mp->napi, skb);
>>
>> continue;
>
> I remember having experimented with this on 3.6 a few months ago with this
> driver and finally switching back to something like this instead which
> showed better performance on my tests :
>
> if (skb->ip_summed == CHECKSUM_UNNECESSARY)
> napi_gro_receive(napi, skb);
> else
> netif_receive_skb(skb);
>
> Unfortunately I don't have more details as my commit message was rather
> short due to this resulting from experimentation. Did you verify that
> you did not lose any performance in various workloads ? I was playing
> with bridges at this time, it's possible that I got better performance
> on bridging with netif_receive_skb() than with napi_gro_receive().
Hi Willy,
I did some simple tests on Dove/Cubox with 'netperf -cCD' and
gso/gro/lro options on
mv643xx_eth. The tests may not be sufficient, as I am not that into
net performance
testing.
I tried todays net-next on top of 3.9-rc6 without any gro patch, with
the initial
patch (Soeren) and your proposed patch (Willy). The results show that
both patches
allow a significant increase in throughput compared to
netif_receive_skb (!gro, !lro)
alone. Having gro with lro disabled gives some 2% more throughput
compared to lro only.
Sebastian
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 16384 16384 10.02 615.65 19.15 99.90 5.097
13.293 (3.9-rc6: gso)
87380 16384 16384 10.02 615.82 19.05 99.90 5.067
13.289 (3.9-rc6: gso, gro)
87380 16384 16384 10.03 747.44 23.17 99.80 5.079
10.938 (3.9-rc6: gso, lro)
87380 16384 16384 10.02 745.28 22.57 99.80 4.963
10.970 (3.9.rc6: gso, gro, lro)
87380 16384 16384 10.02 600.34 19.10 99.90 5.211
13.632 (3.9-rc6+soeren: gso)
87380 16384 16384 10.02 764.23 23.42 99.80 5.021
10.698 (3.9-rc6+soeren: gso, gro)
87380 16384 16384 10.02 749.81 23.13 99.80 5.055
10.904 (3.9-rc6+soeren: gso, lro)
87380 16384 16384 10.02 745.84 22.34 99.80 4.907
10.962 (3.9.rc6+soeren: gso, gro, lro)
87380 16384 16384 10.02 605.79 18.79 100.00 5.082
13.523 (3.9-rc6+willy: gso)
87380 16384 16384 10.02 765.64 24.68 99.80 5.281
10.678 (3.9-rc6+willy: gso, gro)
87380 16384 16384 10.02 750.30 26.02 99.80 5.682
10.897 (3.9-rc6+willy: gso, lro)
87380 16384 16384 10.03 749.40 21.86 99.80 4.778
10.910 (3.9.rc6+willy: gso, gro, lro)
^ permalink raw reply
* [PATCH] add short description of batch mode in tc man page
From: Hubert Kario @ 2013-04-11 14:49 UTC (permalink / raw)
To: netdev, ahu
In-Reply-To: <756691184.2545557.1365691755445.JavaMail.root@redhat.com>
The tc command is missing documentation of -batch and -force switches
that are returned by "tc -help".
Add short description on their syntax and usage.
---
man/man8/tc.8 | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index bbc6790..e0acfeb 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -48,6 +48,9 @@ DEV
.B tc filter show dev
DEV
+.P
+.B tc [ -force ] -b\fR[\fIatch\fR] \fB[ filename ]
+
.ti 8
.IR FORMAT " := {"
\fB\-s\fR[\fItatistics\fR] |
@@ -427,6 +430,15 @@ decode filter offset and mask values to equivalent filter commands based on TCP/
.BR "\-iec"
print rates in IEC units (ie. 1K = 1024).
+.TP
+.BR "\-b", " \-b filename", " \-batch", " \-batch filename"
+read commands from provided file or standard input and invoke them.
+First failure will cause termination of tc.
+
+.TP
+.BR "\-force"
+don't terminate tc on errors in batch mode.
+If there were any errors during execution of the commands, the application return code will be non zero.
.SH HISTORY
.B tc
--
1.8.1.4
--
Regards,
Hubert Kario
Quality Engineer, QE BaseOS Security team
http://wiki.brq.redhat.com/hkario
Email: hkario@redhat.com
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic
^ permalink raw reply related
* Re: [PATCH 11/11] usbnet: handle link change
From: Ming Lei @ 2013-04-11 14:52 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1365691240-816-12-git-send-email-ming.lei@canonical.com>
On Thu, Apr 11, 2013 at 10:40 PM, Ming Lei <ming.lei@canonical.com> wrote:
> The link change is detected via the interrupt pipe, and bulk
> pipes are responsible for transfering packets, so it is reasonable
> to stop bulk transfer after link is reported as off.
>
> Two adavantages may be obtained with stopping bulk transfer
> after link becomes off:
>
> - USB bus bandwidth is saved(USB bus is shared bus except for
> USB3.0), for example, lots of 'IN' token packets and 'NYET'
> handshake packets is transfered on 2.0 bus.
>
> - probabaly power might be saved for usb host controller since
> cancelling bulk transfer may disable the asynchronous schedule of
> host controller.
>
> With this patch, when link becomes off, about ~10% performance
> boost can be found on bulk transfer of anther usb device which
> is attached to same bus with the usbnet device, see below
> test on next-20130410:
>
> - read from usb mass storage(Sandisk Extreme USB 3.0) on pandaboard
> with below command after unplugging ethernet cable:
>
> dd if=/dev/sda iflag=direct of=/dev/null bs=1M count=500
Sorry, the above should be:
dd if=/dev/sda iflag=direct of=/dev/null bs=1M count=800
>
> - without the patch
> 1, 838860800 bytes (839 MB) copied, 36.2216 s, 23.2 MB/s
> 2, 838860800 bytes (839 MB) copied, 35.8368 s, 23.4 MB/s
> 3, 838860800 bytes (839 MB) copied, 35.823 s, 23.4 MB/s
> 4, 838860800 bytes (839 MB) copied, 35.937 s, 23.3 MB/s
> 5, 838860800 bytes (839 MB) copied, 35.7365 s, 23.5 MB/s
> average: 23.6MB/s
>
> - with the patch
> 1, 838860800 bytes (839 MB) copied, 32.3817 s, 25.9 MB/s
> 2, 838860800 bytes (839 MB) copied, 31.7389 s, 26.4 MB/s
> 3, 838860800 bytes (839 MB) copied, 32.438 s, 25.9 MB/s
> 4, 838860800 bytes (839 MB) copied, 32.5492 s, 25.8 MB/s
> 5, 838860800 bytes (839 MB) copied, 31.6178 s, 26.5 MB/s
> average: 26.1MB/s
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/net/usb/usbnet.c | 30 ++++++++++++++++++++++++++++++
> include/linux/usb/usbnet.h | 1 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 34e4252..1e5a9b7 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -938,6 +938,27 @@ static const struct ethtool_ops usbnet_ethtool_ops = {
>
> /*-------------------------------------------------------------------------*/
>
> +static void __handle_link_change(struct usbnet *dev)
> +{
> + if (!test_bit(EVENT_DEV_OPEN, &dev->flags))
> + return;
> +
> + if (!netif_carrier_ok(dev->net)) {
> + /* kill URBs for reading packets to save bus bandwidth */
> + unlink_urbs(dev, &dev->rxq);
> +
> + /*
> + * tx_timeout will unlink URBs for sending packets and
> + * tx queue is stopped by netcore after link becomes off
> + */
> + } else {
> + /* submitting URBs for reading packets */
> + tasklet_schedule(&dev->bh);
> + }
> +
> + clear_bit(EVENT_LINK_CHANGE, &dev->flags);
> +}
> +
> /* work that cannot be done in interrupt context uses keventd.
> *
> * NOTE: with 2.5 we could do more of this using completion callbacks,
> @@ -1035,8 +1056,14 @@ skip_reset:
> } else {
> usb_autopm_put_interface(dev->intf);
> }
> +
> + /* handle link change from link resetting */
> + __handle_link_change(dev);
> }
>
> + if (test_bit (EVENT_LINK_CHANGE, &dev->flags))
> + __handle_link_change(dev);
> +
> if (dev->flags)
> netdev_dbg(dev->net, "kevent done, flags = 0x%lx\n", dev->flags);
> }
> @@ -1286,6 +1313,7 @@ static void usbnet_bh (unsigned long param)
> // or are we maybe short a few urbs?
> } else if (netif_running (dev->net) &&
> netif_device_present (dev->net) &&
> + netif_carrier_ok(dev->net) &&
> !timer_pending (&dev->delay) &&
> !test_bit (EVENT_RX_HALT, &dev->flags)) {
> int temp = dev->rxq.qlen;
> @@ -1663,6 +1691,8 @@ void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset)
>
> if (need_reset && link)
> usbnet_defer_kevent(dev, EVENT_LINK_RESET);
> + else
> + usbnet_defer_kevent(dev, EVENT_LINK_CHANGE);
> }
> EXPORT_SYMBOL(usbnet_link_change);
>
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index eb021b8..da46327 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -72,6 +72,7 @@ struct usbnet {
> # define EVENT_DEVICE_REPORT_IDLE 8
> # define EVENT_NO_RUNTIME_PM 9
> # define EVENT_RX_KILL 10
> +# define EVENT_LINK_CHANGE 11
> };
>
> static inline struct usb_driver *driver_of(struct usb_interface *intf)
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCH] net: mv643xx_eth: Add GRO support
From: Willy Tarreau @ 2013-04-11 15:03 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Andrew Lunn, Jason Cooper, Benjamin Herrenschmidt, linux-kernel,
David S. Miller, Soeren Moch, Paul Mackerras, linux-arm-kernel,
Dale Farnsworth, netdev, linuxppc-dev, Florian Fainelli,
Lennert Buytenhek
In-Reply-To: <CABJ1b_TQDabT1J7ffo5cb1Th=71bcSduifMJaDpfVRiiHvtWDw@mail.gmail.com>
Hi Sebastian,
On Thu, Apr 11, 2013 at 04:47:49PM +0200, Sebastian Hesselbarth wrote:
> I did some simple tests on Dove/Cubox with 'netperf -cCD' and
> gso/gro/lro options on
> mv643xx_eth. The tests may not be sufficient, as I am not that into
> net performance testing.
In fact the difference only happens when the NIC has not verified the
checksum itself IIRC, which should be for non-IPv4 traffic. I agree
that it's not easy to test a bridge with a cubox which has a single
port :-) Maybe you'll see a difference in IPv6 traffic or with VLAN
traffic, as I seem to remember this chip does not do cksum offloading
on VLANs, but I could be wrong.
> I tried todays net-next on top of 3.9-rc6 without any gro patch, with
> the initial
> patch (Soeren) and your proposed patch (Willy). The results show that
> both patches
> allow a significant increase in throughput compared to
> netif_receive_skb (!gro, !lro)
> alone. Having gro with lro disabled gives some 2% more throughput
> compared to lro only.
Indeed this is consistent with my memories, since Eric improved the
GRO path, it became faster than LRO on this chip.
Regards,
Willy
^ permalink raw reply
* Re: [PATCH 00/11] usbnet: usbnet: handle link change
From: Jussi Kivilinna @ 2013-04-11 15:18 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum, netdev,
linux-usb
In-Reply-To: <1365691240-816-1-git-send-email-ming.lei@canonical.com>
On 11.04.2013 17:40, Ming Lei wrote:
> Hi,
>
> This patch set introduces usbnet_link_change() API and applies
> it on all usbnet drivers, then handle the link change centrally
> to stop bulk transfer when link becomes off and restart bulk
> transfer when link becomes on.
Should 'rndis_wlan' be changed to use this too?
-Jussi
>
> With the change, ~10% performance boost on bulk transfer
> of another device on the same bus can be obtained when link
> is off. Also, stopping bulk transfer when link becomes off
> may disable asynchonous schedule of host controller, power
> might be saved probabally.
>
> drivers/net/usb/asix_devices.c | 6 +-----
> drivers/net/usb/ax88179_178a.c | 12 ++++-------
> drivers/net/usb/cdc_ether.c | 5 +----
> drivers/net/usb/cdc_ncm.c | 9 +++-----
> drivers/net/usb/dm9601.c | 7 +------
> drivers/net/usb/mcs7830.c | 6 +-----
> drivers/net/usb/sierra_net.c | 3 +--
> drivers/net/usb/usbnet.c | 45 +++++++++++++++++++++++++++++++++++++++-
> include/linux/usb/usbnet.h | 2 ++
> 9 files changed, 58 insertions(+), 37 deletions(-)
>
>
> Thanks,
> --
> Ming Lei
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 08/11] usbnet: dm9601: apply usbnet_link_change
From: Peter Korsgaard @ 2013-04-11 15:18 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1365691240-816-9-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>>>>> "Ming" == Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
Ming> Use usbnet_link_change to handle link change centrally.
Acked-by: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
Ming> Cc: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
Ming> Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Ming> ---
Ming> drivers/net/usb/dm9601.c | 7 +------
Ming> 1 file changed, 1 insertion(+), 6 deletions(-)
Ming> diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
Ming> index 174e5ec..2dbb946 100644
Ming> --- a/drivers/net/usb/dm9601.c
Ming> +++ b/drivers/net/usb/dm9601.c
Ming> @@ -524,12 +524,7 @@ static void dm9601_status(struct usbnet *dev, struct urb *urb)
Ming> link = !!(buf[0] & 0x40);
Ming> if (netif_carrier_ok(dev->net) != link) {
Ming> - if (link) {
Ming> - netif_carrier_on(dev->net);
Ming> - usbnet_defer_kevent (dev, EVENT_LINK_RESET);
Ming> - }
Ming> - else
Ming> - netif_carrier_off(dev->net);
Ming> + usbnet_link_change(dev, link, 1);
Ming> netdev_dbg(dev->net, "Link Status is: %d\n", link);
Ming> }
Ming> }
Ming> --
Ming> 1.7.9.5
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH RFC] spinlock: split out debugging check from spin_lock_mutex
From: Neil Horman @ 2013-04-11 15:18 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Neil Horman, David Miller, netdev
In-Reply-To: <5166BDAA.3000603@acm.org>
Bart, this patch should fix your problem. Could you please test it and confirm?
Bart Van Assche recently reported a warning to me:
<IRQ> [<ffffffff8103d79f>] warn_slowpath_common+0x7f/0xc0
[<ffffffff8103d7fa>] warn_slowpath_null+0x1a/0x20
[<ffffffff814761dd>] mutex_trylock+0x16d/0x180
[<ffffffff813968c9>] netpoll_poll_dev+0x49/0xc30
[<ffffffff8136a2d2>] ? __alloc_skb+0x82/0x2a0
[<ffffffff81397715>] netpoll_send_skb_on_dev+0x265/0x410
[<ffffffff81397c5a>] netpoll_send_udp+0x28a/0x3a0
[<ffffffffa0541843>] ? write_msg+0x53/0x110 [netconsole]
[<ffffffffa05418bf>] write_msg+0xcf/0x110 [netconsole]
[<ffffffff8103eba1>] call_console_drivers.constprop.17+0xa1/0x1c0
[<ffffffff8103fb76>] console_unlock+0x2d6/0x450
[<ffffffff8104011e>] vprintk_emit+0x1ee/0x510
[<ffffffff8146f9f6>] printk+0x4d/0x4f
[<ffffffffa0004f1d>] scsi_print_command+0x7d/0xe0 [scsi_mod]
This resulted from my commit ca99ca14c which introduced a mutex_trylock
operation in a path that could execute in interrupt context. When mutex
debugging is enabled, the above warns the user when we are in fact exectuting in
interrupt context.
I think this is a false positive however. The check is intended to catch users
who might be issuing sleeping calls in irq context, but the use of mutex_trylock
here is guaranteed not to sleep.
We could fix this by replacing the DEBUG_LOCK_WARN_ON check in spin_lock_mutex
with a __might_sleep call in the appropriate parent mutex operations, but for
the sake of effiency (which It seems is why the check was put in the spin lock
code only when debug is enabled), lets split the spin_lock_mutex call into two
components, where the outer component does the debug checking. Then
mutex_trylock can just call the inner part as its callable from irq context
safely.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Bart Van Assche <bvanassche@acm.org>
CC: Bart Van Assche <bvanassche@acm.org>
CC: David Miller <davem@davemloft.net>
CC: netdev@vger.kernel.org
---
kernel/mutex-debug.h | 11 +++++++++--
kernel/mutex.c | 4 ++--
kernel/mutex.h | 2 ++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/kernel/mutex-debug.h b/kernel/mutex-debug.h
index 0799fd3..bd0d80a 100644
--- a/kernel/mutex-debug.h
+++ b/kernel/mutex-debug.h
@@ -37,13 +37,20 @@ static inline void mutex_clear_owner(struct mutex *lock)
lock->owner = NULL;
}
-#define spin_lock_mutex(lock, flags) \
+#define spin_lock_mutex_raw(lock, flags) \
do { \
struct mutex *l = container_of(lock, struct mutex, wait_lock); \
\
- DEBUG_LOCKS_WARN_ON(in_interrupt()); \
local_irq_save(flags); \
arch_spin_lock(&(lock)->rlock.raw_lock);\
+ } while (0)
+
+#define spin_lock_mutex(lock, flags) \
+ do { \
+ struct mutex *l = container_of(lock, struct mutex, wait_lock); \
+ \
+ DEBUG_LOCKS_WARN_ON(in_interrupt()); \
+ spin_lock_mutex_raw(lock, flags); \
DEBUG_LOCKS_WARN_ON(l->magic != l); \
} while (0)
diff --git a/kernel/mutex.c b/kernel/mutex.c
index 52f2301..5c012c7 100644
--- a/kernel/mutex.c
+++ b/kernel/mutex.c
@@ -431,7 +431,7 @@ static inline int __mutex_trylock_slowpath(atomic_t *lock_count)
unsigned long flags;
int prev;
- spin_lock_mutex(&lock->wait_lock, flags);
+ spin_lock_mutex_raw(&lock->wait_lock, flags);
prev = atomic_xchg(&lock->count, -1);
if (likely(prev == 1)) {
@@ -443,7 +443,7 @@ static inline int __mutex_trylock_slowpath(atomic_t *lock_count)
if (likely(list_empty(&lock->wait_list)))
atomic_set(&lock->count, 0);
- spin_unlock_mutex(&lock->wait_lock, flags);
+ spin_unlock_mutex_raw(&lock->wait_lock, flags);
return prev == 1;
}
diff --git a/kernel/mutex.h b/kernel/mutex.h
index 4115fbf..af6ffb4 100644
--- a/kernel/mutex.h
+++ b/kernel/mutex.h
@@ -11,6 +11,8 @@
#define spin_lock_mutex(lock, flags) \
do { spin_lock(lock); (void)(flags); } while (0)
+#define spin_lock_mutex_raw(lock, flags) spin_lock_mutex((lock), (flags))
+
#define spin_unlock_mutex(lock, flags) \
do { spin_unlock(lock); (void)(flags); } while (0)
#define mutex_remove_waiter(lock, waiter, ti) \
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH iproute2 v2] vxlan: Allow setting destination to unicast address.
From: Stephen Hemminger @ 2013-04-11 15:24 UTC (permalink / raw)
To: Atzm Watanabe; +Cc: netdev
In-Reply-To: <87li8pmkvh.wl%atzm@stratosphere.co.jp>
On Thu, 11 Apr 2013 19:17:54 +0900
Atzm Watanabe <atzm@stratosphere.co.jp> wrote:
> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> index 40167af..0bf03dc 100644
> --- a/include/linux/if_link.h
> +++ b/include/linux/if_link.h
> @@ -296,6 +296,7 @@ enum {
> IFLA_VXLAN_GROUP,
> IFLA_VXLAN_LINK,
> IFLA_VXLAN_LOCAL,
> + IFLA_VXLAN_REMOTE,
> IFLA_VXLAN_TTL,
> IFLA_VXLAN_TOS,
You can't insert another value into the middle of an enum list.
It changes the kernel-userspace ABI values.
^ permalink raw reply
* [PATCH for-3.9] pci: avoid work_on_cpu for nested SRIOV probes
From: Michael S. Tsirkin @ 2013-04-11 15:25 UTC (permalink / raw)
To: Or Gerlitz
Cc: Ming Lei, Greg Kroah-Hartman, David Miller, Roland Dreier, netdev,
Yan Burman, Jack Morgenstein
In-Reply-To: <51360D7C.3060209@mellanox.com>
The following lockdep report triggers since 3.9-rc1:
=============================================
[ INFO: possible recursive locking detected ]
3.9.0-rc1 #96 Not tainted
---------------------------------------------
kworker/0:1/734 is trying to acquire lock:
((&wfc.work)){+.+.+.}, at: [<ffffffff81066cb0>] flush_work+0x0/0x250
but task is already holding lock:
((&wfc.work)){+.+.+.}, at: [<ffffffff81064352>]
process_one_work+0x162/0x4c0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock((&wfc.work));
lock((&wfc.work));
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by kworker/0:1/734:
#0: (events){.+.+.+}, at: [<ffffffff81064352>]
process_one_work+0x162/0x4c0
#1: ((&wfc.work)){+.+.+.}, at: [<ffffffff81064352>]
process_one_work+0x162/0x4c0
#2: (&__lockdep_no_validate__){......}, at: [<ffffffff812db225>]
device_attach+0x25/0xb0
stack backtrace:
Pid: 734, comm: kworker/0:1 Not tainted 3.9.0-rc1 #96
Call Trace:
[<ffffffff810948ec>] validate_chain+0xdcc/0x11f0
[<ffffffff81095150>] __lock_acquire+0x440/0xc70
[<ffffffff81095150>] ? __lock_acquire+0x440/0xc70
[<ffffffff810959da>] lock_acquire+0x5a/0x70
[<ffffffff81066cb0>] ? wq_worker_waking_up+0x60/0x60
[<ffffffff81066cf5>] flush_work+0x45/0x250
[<ffffffff81066cb0>] ? wq_worker_waking_up+0x60/0x60
[<ffffffff810922be>] ? mark_held_locks+0x9e/0x130
[<ffffffff81066a96>] ? queue_work_on+0x46/0x90
[<ffffffff810925dd>] ? trace_hardirqs_on_caller+0xfd/0x190
[<ffffffff8109267d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff81066f74>] work_on_cpu+0x74/0x90
[<ffffffff81063820>] ? keventd_up+0x20/0x20
[<ffffffff8121fd30>] ? pci_pm_prepare+0x60/0x60
[<ffffffff811f9293>] ? cpumask_next_and+0x23/0x40
[<ffffffff81220a1a>] pci_device_probe+0xba/0x110
[<ffffffff812dadca>] ? driver_sysfs_add+0x7a/0xb0
[<ffffffff812daf1f>] driver_probe_device+0x8f/0x230
[<ffffffff812db170>] ? __driver_attach+0xb0/0xb0
[<ffffffff812db1bb>] __device_attach+0x4b/0x60
[<ffffffff812d9314>] bus_for_each_drv+0x64/0x90
[<ffffffff812db298>] device_attach+0x98/0xb0
[<ffffffff81218474>] pci_bus_add_device+0x24/0x50
[<ffffffff81232e80>] virtfn_add+0x240/0x3e0
[<ffffffff8146ce3d>] ? _raw_spin_unlock_irqrestore+0x3d/0x80
[<ffffffff812333be>] pci_enable_sriov+0x23e/0x500
[<ffffffffa011fa1a>] __mlx4_init_one+0x5da/0xce0 [mlx4_core]
[<ffffffffa012016d>] mlx4_init_one+0x2d/0x60 [mlx4_core]
[<ffffffff8121fd79>] local_pci_probe+0x49/0x80
[<ffffffff81063833>] work_for_cpu_fn+0x13/0x20
[<ffffffff810643b8>] process_one_work+0x1c8/0x4c0
[<ffffffff81064352>] ? process_one_work+0x162/0x4c0
[<ffffffff81064cfb>] worker_thread+0x30b/0x430
[<ffffffff810649f0>] ? manage_workers+0x340/0x340
[<ffffffff8106cea6>] kthread+0xd6/0xe0
[<ffffffff8106cdd0>] ? __init_kthread_worker+0x70/0x70
[<ffffffff8146daac>] ret_from_fork+0x7c/0xb0
[<ffffffff8106cdd0>] ? __init_kthread_worker+0x70/0x70
The issue is that a driver, in it's probe function, calls
pci_sriov_enable so a PF device probe causes VF probe (AKA nested
probe). Each probe in pci_device_probe which is (normally) run through
work_on_cpu (this is to get the right numa node for memory allocated by
the driver). In turn work_on_cpu does this internally:
schedule_work_on(cpu, &wfc.work);
flush_work(&wfc.work);
So if you are running probe on CPU1, and cause another
probe on the same CPU, this will try to flush
workqueue from inside same workqueue which of course
deadlocks.
Nested probing might be tricky to get right generally.
But for pci_sriov_enable, the situation is actually very simple: all VFs
naturally have same affinity as the PF, and cpumask_any_and is actually
same as cpumask_first_and, so it always gives us the same CPU.
So let's just detect that, and run the probing for VFs locally without a
workqueue.
This is hardly elegant, but looks to me like an appropriate quick fix
for 3.9.
Tested-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 1fa1e48..6eeb5ec 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -286,8 +286,8 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
int cpu;
get_online_cpus();
cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
- if (cpu < nr_cpu_ids)
+ if (cpu != raw_smp_processor_id() && cpu < nr_cpu_ids)
error = work_on_cpu(cpu, local_pci_probe, &ddi);
else
error = local_pci_probe(&ddi);
^ permalink raw reply related
* Re: [PATCH] net: mv643xx_eth: Add GRO support
From: Sebastian Hesselbarth @ 2013-04-11 15:27 UTC (permalink / raw)
To: Willy Tarreau
Cc: Andrew Lunn, Jason Cooper, Benjamin Herrenschmidt, linux-kernel,
Florian Fainelli, Soeren Moch, Paul Mackerras, Lennert Buytenhek,
Dale Farnsworth, netdev, linuxppc-dev, David S. Miller,
linux-arm-kernel
In-Reply-To: <20130411150326.GA19978@1wt.eu>
On Thu, Apr 11, 2013 at 5:03 PM, Willy Tarreau <w@1wt.eu> wrote:
> On Thu, Apr 11, 2013 at 04:47:49PM +0200, Sebastian Hesselbarth wrote:
>> I tried todays net-next on top of 3.9-rc6 without any gro patch, with
>> the initial
>> patch (Soeren) and your proposed patch (Willy). The results show that
>> both patches
>> allow a significant increase in throughput compared to
>> netif_receive_skb (!gro, !lro)
>> alone. Having gro with lro disabled gives some 2% more throughput
>> compared to lro only.
>
> Indeed this is consistent with my memories, since Eric improved the
> GRO path, it became faster than LRO on this chip.
I don't have a strong opinion on whether Soeren's or your proposal should
be submitted. But I insist on having one of them in, as GRO significantly
improves the common use case, is enabled by default, and not as
constrained as LRO.
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