The following reply was made to PR kern/180791; it has been noted by GNATS.
From: Shahar Klein <shah...@mellanox.com>
To: John Baldwin <j...@freebsd.org>, "bug-follo...@freebsd.org"
<bug-follo...@freebsd.org>
Cc: Oded Shanoon <od...@mellanox.com>
Subject: RE: kern/180791: [ofed] [patch] Kernel crash on ifdown and
kldunload mlxen
Date: Mon, 29 Jul 2013 06:46:37 +0000
Thanks John,
I've reviewed and tested this patch and it's working fine.
Is this patch going to both 10.0 and 9.2?
10x
Shahar
-----Original Message-----
From: John Baldwin [mailto:j...@freebsd.org]=20
Sent: Thursday, July 25, 2013 10:39 PM
To: bug-follo...@freebsd.org; Shahar Klein
Subject: Re: kern/180791: [ofed] [patch] Kernel crash on ifdown and kldunlo=
ad mlxen
Thanks. One note is that it seems like the state_lock should be held when =
stop is called. Also, the callout used for stats should be drained during =
detach. (If you use callot_init_mtx() instead of callout_init() then
callout_stop() under the lock is race-free, though I'd still do the
callout_drain() during detach to be safe.)
Also, I had some other changes I made to this file to make the locking more=
consistent with other NIC drivers in the tree. Can you look at this and t=
est this patch please?
Index: en_netdev.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- en_netdev.c (revision 253547)
+++ en_netdev.c (working copy)
@@ -495,11 +495,6 @@ static void mlx4_en_do_get_stats(struct work_struc
=20
queue_delayed_work(mdev->workqueue, &priv->stats_task,
STATS_DELAY);
}
- if (mdev->mac_removed[MLX4_MAX_PORTS + 1 - priv->port]) {
- panic("mlx4_en_do_get_stats: Unexpected mac removed for %d\n",
- priv->port);
- mdev->mac_removed[MLX4_MAX_PORTS + 1 - priv->port] =3D 0;
- }
mutex_unlock(&mdev->state_lock);
}
=20
@@ -688,8 +683,8 @@ int mlx4_en_start_port(struct net_device *dev)
mlx4_en_set_multicast(dev);
=20
/* Enable the queues. */
- atomic_clear_int(&dev->if_drv_flags, IFF_DRV_OACTIVE);
- atomic_set_int(&dev->if_drv_flags, IFF_DRV_RUNNING);
+ dev->if_drv_flags &=3D ~IFF_DRV_OACTIVE;
+ dev->if_drv_flags |=3D IFF_DRV_RUNNING;
=20
callout_reset(&priv->watchdog_timer, MLX4_EN_WATCHDOG_TIMEOUT,
mlx4_en_watchdog_timeout, priv);
@@ -761,7 +756,7 @@ void mlx4_en_stop_port(struct net_device *dev)
=20
callout_stop(&priv->watchdog_timer);
=20
- atomic_clear_int(&dev->if_drv_flags, IFF_DRV_RUNNING);
+ dev->if_drv_flags &=3D ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
}
=20
static void mlx4_en_restart(struct work_struct *work) @@ -802,19 +797,30 @=
@ mlx4_en_init(void *arg) {
struct mlx4_en_priv *priv;
struct mlx4_en_dev *mdev;
+
+ priv =3D arg;
+ mdev =3D priv->mdev;
+ mutex_lock(&mdev->state_lock);
+ mlx4_en_init_locked(priv);
+ mutex_unlock(&mdev->state_lock);
+}
+
+static void
+mlx4_en_init_locked(struct mlx4_en_priv *priv) {
+
+ struct mlx4_en_dev *mdev;
struct ifnet *dev;
int i;
=20
- priv =3D arg;
dev =3D priv->dev;
mdev =3D priv->mdev;
- mutex_lock(&mdev->state_lock);
if (dev->if_drv_flags & IFF_DRV_RUNNING)
mlx4_en_stop_port(dev);
=20
if (!mdev->device_up) {
en_err(priv, "Cannot open - device down/disabled\n");
- goto out;
+ return;
}
=20
/* Reset HW statistics and performance counters */ @@ -835,9 +841,6 @@
ml=
x4_en_init(void *arg)
mlx4_en_set_default_moderation(priv);
if (mlx4_en_start_port(dev))
en_err(priv, "Failed starting port:%d\n", priv->port);
-
-out:
- mutex_unlock(&mdev->state_lock);
}
=20
void mlx4_en_free_resources(struct mlx4_en_priv *priv) @@ -927,9 +930,14 @=
@ void mlx4_en_destroy_netdev(struct net_device *dev
if (priv->sysctl)
sysctl_ctx_free(&priv->conf_ctx);
=20
+ mutex_lock(&mdev->state_lock);
+ mlx4_en_stop_port(dev);
+ mutex_unlock(&mdev->state_lock);
+
cancel_delayed_work(&priv->stats_task);
/* flush any pending task for this netdev */
flush_workqueue(mdev->workqueue);
+ callout_drain(&priv->watchdog_timer);
=20
/* Detach the netdev so tasks would not attempt to access it */
mutex_lock(&mdev->state_lock);
@@ -1091,25 +1099,25 @@ static int mlx4_en_ioctl(struct ifnet *dev, u_long
error =3D -mlx4_en_change_mtu(dev, ifr->ifr_mtu);
break;
case SIOCSIFFLAGS:
+ mutex_lock(&mdev->state_lock);
if (dev->if_flags & IFF_UP) {
- if ((dev->if_drv_flags & IFF_DRV_RUNNING) =3D=3D 0) {
- mutex_lock(&mdev->state_lock);
+ if ((dev->if_drv_flags & IFF_DRV_RUNNING) =3D=3D 0)
mlx4_en_start_port(dev);
- mutex_unlock(&mdev->state_lock);
- } else
+ else
mlx4_en_set_multicast(dev);
} else {
- mutex_lock(&mdev->state_lock);
if (dev->if_drv_flags & IFF_DRV_RUNNING) {
mlx4_en_stop_port(dev);
if_link_state_change(dev, LINK_STATE_DOWN);
}
- mutex_unlock(&mdev->state_lock);
}
+ mutex_unlock(&mdev->state_lock);
break;
case SIOCADDMULTI:
case SIOCDELMULTI:
+ mutex_lock(&mdev->state_lock);
mlx4_en_set_multicast(dev);
+ mutex_unlock(&mdev->state_lock);
break;
case SIOCSIFMEDIA:
case SIOCGIFMEDIA:
@@ -1116,6 +1124,7 @@ static int mlx4_en_ioctl(struct ifnet *dev, u_long
error =3D ifmedia_ioctl(dev, ifr, &priv->media, command);
break;
case SIOCSIFCAP:
+ mutex_lock(&mdev->state_lock);
mask =3D ifr->ifr_reqcap ^ dev->if_capenable;
if (mask & IFCAP_HWCSUM)
dev->if_capenable ^=3D IFCAP_HWCSUM;
@@ -1130,7 +1139,8 @@ static int mlx4_en_ioctl(struct ifnet *dev, u_long
if (mask & IFCAP_WOL_MAGIC)
dev->if_capenable ^=3D IFCAP_WOL_MAGIC;
if (dev->if_drv_flags & IFF_DRV_RUNNING)
- mlx4_en_init(priv);
+ mlx4_en_init_locked(priv);
+ mutex_unlock(&mdev->state_lock);
VLAN_CAPABILITIES(dev);
break;
default:
--
John Baldwin
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
