Merge branch 'netvsc-lockdep-and-related-fixes'

Stephen Hemminger says: ==================== netvsc: lockdep and related fixes These fix sparse and lockdep warnings from netvsc driver. Targeting these at net-next since no actual related failures have been observed in non-debug kernels. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2017-07-19 22:20:05 -07:00 · 2017-07-19 22:20:05 -07:00 · 88f34810e3
commit 88f34810e3
parent 9492f4269e 3962981f48
4 changed files with 107 additions and 97 deletions
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@ -183,10 +183,12 @@ struct rndis_device {
 /* Interface */
 struct rndis_message;
 struct netvsc_device;
-int netvsc_device_add(struct hv_device *device,
-		      const struct netvsc_device_info *info);
+struct net_device_context;
+
+struct netvsc_device *netvsc_device_add(struct hv_device *device,
+					const struct netvsc_device_info *info);
 void netvsc_device_remove(struct hv_device *device);
-int netvsc_send(struct hv_device *device,
+int netvsc_send(struct net_device_context *ndc,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
 		struct hv_page_buffer **page_buffer,
@ -200,10 +202,11 @@ int netvsc_recv_callback(struct net_device *net,
 			 const struct ndis_pkt_8021q_info *vlan);
 void netvsc_channel_cb(void *context);
 int netvsc_poll(struct napi_struct *napi, int budget);
+bool rndis_filter_opened(const struct netvsc_device *nvdev);
 int rndis_filter_open(struct netvsc_device *nvdev);
 int rndis_filter_close(struct netvsc_device *nvdev);
-int rndis_filter_device_add(struct hv_device *dev,
-			    struct netvsc_device_info *info);
+struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
+					      struct netvsc_device_info *info);
 void rndis_filter_update(struct netvsc_device *nvdev);
 void rndis_filter_device_remove(struct hv_device *dev,
 				struct netvsc_device *nvdev);
@ -724,6 +727,7 @@ struct net_device_context {
 /* Per channel data */
 struct netvsc_channel {
 	struct vmbus_channel *channel;
+	struct netvsc_device *net_device;
 	const struct vmpacket_descriptor *desc;
 	struct napi_struct napi;
 	struct multi_send_data msd;
@ -783,18 +787,6 @@ struct netvsc_device {
 	struct rcu_head rcu;
 };

-static inline struct netvsc_device *
-net_device_to_netvsc_device(struct net_device *ndev)
-{
-	return ((struct net_device_context *)netdev_priv(ndev))->nvdev;
-}
-
-static inline struct netvsc_device *
-hv_device_to_netvsc_device(struct hv_device *device)
-{
-	return net_device_to_netvsc_device(hv_get_drvdata(device));
-}
-
 /* NdisInitialize message */
 struct rndis_initialize_request {
 	u32 req_id;
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@ -29,6 +29,8 @@
 #include <linux/netdevice.h>
 #include <linux/if_ether.h>
 #include <linux/vmalloc.h>
+#include <linux/rtnetlink.h>
+
 #include <asm/sync_bitops.h>

 #include "hyperv_net.h"
@ -41,7 +43,7 @@ void netvsc_switch_datapath(struct net_device *ndev, bool vf)
 {
 	struct net_device_context *net_device_ctx = netdev_priv(ndev);
 	struct hv_device *dev = net_device_ctx->device_ctx;
-	struct netvsc_device *nv_dev = net_device_ctx->nvdev;
+	struct netvsc_device *nv_dev = rtnl_dereference(net_device_ctx->nvdev);
 	struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;

 	memset(init_pkt, 0, sizeof(struct nvsp_message));
@ -103,7 +105,8 @@ static void netvsc_destroy_buf(struct hv_device *device)
 {
 	struct nvsp_message *revoke_packet;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct netvsc_device *net_device = net_device_to_netvsc_device(ndev);
+	struct net_device_context *ndc = netdev_priv(ndev);
+	struct netvsc_device *net_device = rtnl_dereference(ndc->nvdev);
 	int ret;

 	/*
@ -549,7 +552,8 @@ void netvsc_device_remove(struct hv_device *device)
 {
 	struct net_device *ndev = hv_get_drvdata(device);
 	struct net_device_context *net_device_ctx = netdev_priv(ndev);
-	struct netvsc_device *net_device = net_device_ctx->nvdev;
+	struct netvsc_device *net_device
+		= rtnl_dereference(net_device_ctx->nvdev);
 	int i;

 	netvsc_disconnect_vsp(device);
@ -819,13 +823,16 @@ static inline void move_pkt_msd(struct hv_netvsc_packet **msd_send,
 	msdp->count = 0;
 }

-int netvsc_send(struct hv_device *device,
+/* RCU already held by caller */
+int netvsc_send(struct net_device_context *ndev_ctx,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
 		struct hv_page_buffer **pb,
 		struct sk_buff *skb)
 {
-	struct netvsc_device *net_device = hv_device_to_netvsc_device(device);
+	struct netvsc_device *net_device
+		= rcu_dereference_rtnl(ndev_ctx->nvdev);
+	struct hv_device *device = ndev_ctx->device_ctx;
 	int ret = 0;
 	struct netvsc_channel *nvchan;
 	u32 pktlen = packet->total_data_buflen, msd_len = 0;
@ -837,7 +844,7 @@ int netvsc_send(struct hv_device *device,
 	bool xmit_more = (skb != NULL) ? skb->xmit_more : false;

 	/* If device is rescinded, return error and packet will get dropped. */
-	if (unlikely(net_device->destroy))
+	if (unlikely(!net_device || net_device->destroy))
 		return -ENODEV;

 	/* We may race with netvsc_connect_vsp()/netvsc_init_buf() and get
@ -1219,11 +1226,11 @@ int netvsc_poll(struct napi_struct *napi, int budget)
 {
 	struct netvsc_channel *nvchan
 		= container_of(napi, struct netvsc_channel, napi);
+	struct netvsc_device *net_device = nvchan->net_device;
 	struct vmbus_channel *channel = nvchan->channel;
 	struct hv_device *device = netvsc_channel_to_device(channel);
 	u16 q_idx = channel->offermsg.offer.sub_channel_index;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct netvsc_device *net_device = net_device_to_netvsc_device(ndev);
 	int work_done = 0;

 	/* If starting a new interval */
@ -1271,8 +1278,8 @@ void netvsc_channel_cb(void *context)
 * netvsc_device_add - Callback when the device belonging to this
 * driver is added
 */
-int netvsc_device_add(struct hv_device *device,
-		      const struct netvsc_device_info *device_info)
+struct netvsc_device *netvsc_device_add(struct hv_device *device,
+				const struct netvsc_device_info *device_info)
 {
 	int i, ret = 0;
 	int ring_size = device_info->ring_size;
@ -1282,7 +1289,7 @@ int netvsc_device_add(struct hv_device *device,

 	net_device = alloc_net_device();
 	if (!net_device)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);

 	net_device->ring_size = ring_size;

@ -1302,6 +1309,7 @@ int netvsc_device_add(struct hv_device *device,
 		struct netvsc_channel *nvchan = &net_device->chan_table[i];

 		nvchan->channel = device->channel;
+		nvchan->net_device = net_device;
 	}

 	/* Enable NAPI handler before init callbacks */
@ -1338,7 +1346,7 @@ int netvsc_device_add(struct hv_device *device,
 		goto close;
 	}

-	return ret;
+	return net_device;

 close:
 	netif_napi_del(&net_device->chan_table[0].napi);
@ -1349,6 +1357,5 @@ close:
 cleanup:
 	free_netvsc_device(&net_device->rcu);

-	return ret;
-
+	return ERR_PTR(ret);
 }
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@ -69,7 +69,7 @@ static void netvsc_set_multicast_list(struct net_device *net)
 static int netvsc_open(struct net_device *net)
 {
 	struct net_device_context *ndev_ctx = netdev_priv(net);
-	struct netvsc_device *nvdev = ndev_ctx->nvdev;
+	struct netvsc_device *nvdev = rtnl_dereference(ndev_ctx->nvdev);
 	struct rndis_device *rdev;
 	int ret = 0;

@ -505,8 +505,8 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)

 	/* timestamp packet in software */
 	skb_tx_timestamp(skb);
-	ret = netvsc_send(net_device_ctx->device_ctx, packet,
-			  rndis_msg, &pb, skb);
+
+	ret = netvsc_send(net_device_ctx, packet, rndis_msg, &pb, skb);
 	if (likely(ret == 0))
 		return NETDEV_TX_OK;

@ -717,6 +717,7 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
 			     u32 num_chn)
 {
 	struct netvsc_device_info device_info;
+	struct netvsc_device *net_device;
 	int ret;

 	memset(&device_info, 0, sizeof(device_info));
@ -724,17 +725,16 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
 	device_info.ring_size = ring_size;
 	device_info.max_num_vrss_chns = num_chn;

-	ret = rndis_filter_device_add(dev, &device_info);
-	if (ret)
-		return ret;
-
 	ret = netif_set_real_num_tx_queues(net, num_chn);
 	if (ret)
 		return ret;

 	ret = netif_set_real_num_rx_queues(net, num_chn);
+	if (ret)
+		return ret;

-	return ret;
+	net_device = rndis_filter_device_add(dev, &device_info);
+	return IS_ERR(net_device) ? PTR_ERR(net_device) : 0;
 }

 static int netvsc_set_channels(struct net_device *net,
@ -744,7 +744,7 @@ static int netvsc_set_channels(struct net_device *net,
 	struct hv_device *dev = net_device_ctx->device_ctx;
 	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 	unsigned int count = channels->combined_count;
-	bool was_running;
+	bool was_opened;
 	int ret;

 	/* We do not support separate count for rx, tx, or other */
@ -764,12 +764,9 @@ static int netvsc_set_channels(struct net_device *net,
 	if (count > nvdev->max_chn)
 		return -EINVAL;

-	was_running = netif_running(net);
-	if (was_running) {
-		ret = netvsc_close(net);
-		if (ret)
-			return ret;
-	}
+	was_opened = rndis_filter_opened(nvdev);
+	if (was_opened)
+		rndis_filter_close(nvdev);

 	rndis_filter_device_remove(dev, nvdev);

@ -779,10 +776,12 @@ static int netvsc_set_channels(struct net_device *net,
 	else
 		netvsc_set_queues(net, dev, nvdev->num_chn);

-	if (was_running)
-		ret = netvsc_open(net);
+	nvdev = rtnl_dereference(net_device_ctx->nvdev);
+	if (was_opened)
+		rndis_filter_open(nvdev);

 	/* We may have missed link change notifications */
+	net_device_ctx->last_reconfig = 0;
 	schedule_delayed_work(&net_device_ctx->dwork, 0);

 	return ret;
@ -848,19 +847,18 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	struct net_device_context *ndevctx = netdev_priv(ndev);
 	struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 	struct hv_device *hdev = ndevctx->device_ctx;
+	int orig_mtu = ndev->mtu;
 	struct netvsc_device_info device_info;
-	bool was_running;
+	bool was_opened;
 	int ret = 0;

 	if (!nvdev || nvdev->destroy)
 		return -ENODEV;

-	was_running = netif_running(ndev);
-	if (was_running) {
-		ret = netvsc_close(ndev);
-		if (ret)
-			return ret;
-	}
+	netif_device_detach(ndev);
+	was_opened = rndis_filter_opened(nvdev);
+	if (was_opened)
+		rndis_filter_close(nvdev);

 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;
@ -869,18 +867,21 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)

 	rndis_filter_device_remove(hdev, nvdev);

-	/* 'nvdev' has been freed in rndis_filter_device_remove() ->
-	 * netvsc_device_remove () -> free_netvsc_device().
-	 * We mustn't access it before it's re-created in
-	 * rndis_filter_device_add() -> netvsc_device_add().
-	 */
-
 	ndev->mtu = mtu;

-	rndis_filter_device_add(hdev, &device_info);
+	nvdev = rndis_filter_device_add(hdev, &device_info);
+	if (IS_ERR(nvdev)) {
+		ret = PTR_ERR(nvdev);

-	if (was_running)
-		ret = netvsc_open(ndev);
+		/* Attempt rollback to original MTU */
+		ndev->mtu = orig_mtu;
+		rndis_filter_device_add(hdev, &device_info);
+	}
+
+	if (was_opened)
+		rndis_filter_open(nvdev);
+
+	netif_device_attach(ndev);

 	/* We may have missed link change notifications */
 	schedule_delayed_work(&ndevctx->dwork, 0);
@ -1363,7 +1364,7 @@ static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
 			continue;	/* not a netvsc device */

 		net_device_ctx = netdev_priv(dev);
-		if (net_device_ctx->nvdev == NULL)
+		if (!rtnl_dereference(net_device_ctx->nvdev))
 			continue;	/* device is removed */

 		if (rtnl_dereference(net_device_ctx->vf_netdev) == vf_netdev)
@ -1528,8 +1529,10 @@ static int netvsc_probe(struct hv_device *dev,
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;
 	device_info.num_chn = VRSS_CHANNEL_DEFAULT;
-	ret = rndis_filter_device_add(dev, &device_info);
-	if (ret != 0) {
+
+	nvdev = rndis_filter_device_add(dev, &device_info);
+	if (IS_ERR(nvdev)) {
+		ret = PTR_ERR(nvdev);
 		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
 		free_netdev(net);
 		hv_set_drvdata(dev, NULL);
@ -1543,10 +1546,11 @@ static int netvsc_probe(struct hv_device *dev,
 		NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 	net->vlan_features = net->features;

-	/* RCU not necessary here, device not registered */
-	nvdev = net_device_ctx->nvdev;
 	netif_set_real_num_tx_queues(net, nvdev->num_chn);
 	netif_set_real_num_rx_queues(net, nvdev->num_chn);
+	rtnl_unlock();
+
+	netdev_lockdep_set_classes(net);

 	/* MTU range: 68 - 1500 or 65521 */
 	net->min_mtu = NETVSC_MTU_MIN;
@ -1588,7 +1592,8 @@ static int netvsc_remove(struct hv_device *dev)
 	 * removed. Also blocks mtu and channel changes.
 	 */
 	rtnl_lock();
-	rndis_filter_device_remove(dev, ndev_ctx->nvdev);
+	rndis_filter_device_remove(dev,
+				   rtnl_dereference(ndev_ctx->nvdev));
 	rtnl_unlock();

 	unregister_netdev(net);
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@ -84,6 +84,14 @@ static struct rndis_device *get_rndis_device(void)
 	return device;
 }

+static struct netvsc_device *
+net_device_to_netvsc_device(struct net_device *ndev)
+{
+	struct net_device_context *net_device_ctx = netdev_priv(ndev);
+
+	return rtnl_dereference(net_device_ctx->nvdev);
+}
+
 static struct rndis_request *get_rndis_request(struct rndis_device *dev,
 					     u32 msg_type,
 					     u32 msg_len)
@ -243,7 +251,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
 			pb[0].len;
 	}

-	ret = netvsc_send(net_device_ctx->device_ctx, packet, NULL, &pb, NULL);
+	ret = netvsc_send(net_device_ctx, packet, NULL, &pb, NULL);
 	return ret;
 }

@ -472,7 +480,7 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid,

 	if (oid == OID_TCP_OFFLOAD_HARDWARE_CAPABILITIES) {
 		struct net_device_context *ndevctx = netdev_priv(dev->ndev);
-		struct netvsc_device *nvdev = ndevctx->nvdev;
+		struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 		struct ndis_offload *hwcaps;
 		u32 nvsp_version = nvdev->nvsp_version;
 		u8 ndis_rev;
@ -658,9 +666,9 @@ cleanup:

 static int
 rndis_filter_set_offload_params(struct net_device *ndev,
+				struct netvsc_device *nvdev,
 				struct ndis_offload_params *req_offloads)
 {
-	struct netvsc_device *nvdev = net_device_to_netvsc_device(ndev);
 	struct rndis_device *rdev = nvdev->extension;
 	struct rndis_request *request;
 	struct rndis_set_request *set;
@ -944,7 +952,7 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 	struct rndis_request *request;
 	struct rndis_halt_request *halt;
 	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
-	struct netvsc_device *nvdev = net_device_ctx->nvdev;
+	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);

 	/* Attempt to do a rndis device halt */
 	request = get_rndis_request(dev, RNDIS_MSG_HALT,
@ -1052,8 +1060,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
 		complete(&nvscdev->channel_init_wait);
 }

-int rndis_filter_device_add(struct hv_device *dev,
-			    struct netvsc_device_info *device_info)
+struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
+				      struct netvsc_device_info *device_info)
 {
 	struct net_device *net = hv_get_drvdata(dev);
 	struct net_device_context *net_device_ctx = netdev_priv(net);
@ -1072,21 +1080,20 @@ int rndis_filter_device_add(struct hv_device *dev,

 	rndis_device = get_rndis_device();
 	if (!rndis_device)
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);

 	/*
 	 * Let the inner driver handle this first to create the netvsc channel
 	 * NOTE! Once the channel is created, we may get a receive callback
 	 * (RndisFilterOnReceive()) before this call is completed
 	 */
-	ret = netvsc_device_add(dev, device_info);
-	if (ret != 0) {
+	net_device = netvsc_device_add(dev, device_info);
+	if (IS_ERR(net_device)) {
 		kfree(rndis_device);
-		return ret;
+		return net_device;
 	}

 	/* Initialize the rndis device */
-	net_device = net_device_ctx->nvdev;
 	net_device->max_chn = 1;
 	net_device->num_chn = 1;

@ -1097,10 +1104,8 @@ int rndis_filter_device_add(struct hv_device *dev,

 	/* Send the rndis initialization message */
 	ret = rndis_filter_init_device(rndis_device);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;

 	/* Get the MTU from the host */
 	size = sizeof(u32);
@ -1112,19 +1117,15 @@ int rndis_filter_device_add(struct hv_device *dev,

 	/* Get the mac address */
 	ret = rndis_filter_query_device_mac(rndis_device);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;

 	memcpy(device_info->mac_adr, rndis_device->hw_mac_adr, ETH_ALEN);

 	/* Find HW offload capabilities */
 	ret = rndis_query_hwcaps(rndis_device, &hwcaps);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;

 	/* A value of zero means "no change"; now turn on what we want. */
 	memset(&offloads, 0, sizeof(struct ndis_offload_params));
@ -1179,7 +1180,7 @@ int rndis_filter_device_add(struct hv_device *dev,

 	netif_set_gso_max_size(net, gso_max_size);

-	ret = rndis_filter_set_offload_params(net, &offloads);
+	ret = rndis_filter_set_offload_params(net, net_device, &offloads);
 	if (ret)
 		goto err_dev_remv;

@ -1190,7 +1191,7 @@ int rndis_filter_device_add(struct hv_device *dev,
 		   rndis_device->link_state ? "down" : "up");

 	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
-		return 0;
+		return net_device;

 	rndis_filter_query_link_speed(rndis_device);

@ -1223,7 +1224,7 @@ int rndis_filter_device_add(struct hv_device *dev,

 	num_rss_qs = net_device->num_chn - 1;
 	if (num_rss_qs == 0)
-		return 0;
+		return net_device;

 	refcount_set(&net_device->sc_offered, num_rss_qs);
 	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
@ -1260,11 +1261,11 @@ out:
 		net_device->num_chn = 1;
 	}

-	return 0; /* return 0 because primary channel can be used alone */
+	return net_device;

 err_dev_remv:
 	rndis_filter_device_remove(dev, net_device);
-	return ret;
+	return ERR_PTR(ret);
 }

 void rndis_filter_device_remove(struct hv_device *dev,
@ -1302,3 +1303,8 @@ int rndis_filter_close(struct netvsc_device *nvdev)

 	return rndis_filter_close_device(nvdev->extension);
 }
+
+bool rndis_filter_opened(const struct netvsc_device *nvdev)
+{
+	return atomic_read(&nvdev->open_cnt) > 0;
+}