diff mbox series

[net-next,1/4] hv_netvsc: netvsc_teardown_gpadl() split

Message ID 20171031134204.15287-2-vkuznets@redhat.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series hv_netvsc: fix some crashes and hangs on channel/mtu changes | expand

Commit Message

Vitaly Kuznetsov Oct. 31, 2017, 1:42 p.m. UTC
It was found that in some cases host refuses to teardown GPADL for send/
receive buffers (probably when some work with these buffere is scheduled or
ongoing). Change the teardown logic to be:
1) Send NVSP_MSG1_TYPE_REVOKE_* messages
2) Close the channel
3) Teardown GPADLs.
This seems to work reliably.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/net/hyperv/netvsc.c | 69 +++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

Comments

Stephen Hemminger Oct. 31, 2017, 4:37 p.m. UTC | #1
On Tue, 31 Oct 2017 14:42:01 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> It was found that in some cases host refuses to teardown GPADL for send/
> receive buffers (probably when some work with these buffere is scheduled or
> ongoing). Change the teardown logic to be:
> 1) Send NVSP_MSG1_TYPE_REVOKE_* messages
> 2) Close the channel
> 3) Teardown GPADLs.
> This seems to work reliably.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  drivers/net/hyperv/netvsc.c | 69 +++++++++++++++++++++++----------------------
>  1 file changed, 36 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 5bb6a20072dd..bfc79698b8f4 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -100,12 +100,11 @@ static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
>  	call_rcu(&nvdev->rcu, free_netvsc_device);
>  }
>  
> -static void netvsc_destroy_buf(struct hv_device *device)
> +static void netvsc_revoke_buf(struct hv_device *device,
> +			      struct netvsc_device *net_device)
>  {
>  	struct nvsp_message *revoke_packet;
>  	struct net_device *ndev = hv_get_drvdata(device);
> -	struct net_device_context *ndc = netdev_priv(ndev);
> -	struct netvsc_device *net_device = rtnl_dereference(ndc->nvdev);
>  	int ret;
>  
>  	/*
> @@ -148,28 +147,6 @@ static void netvsc_destroy_buf(struct hv_device *device)
>  		net_device->recv_section_cnt = 0;
>  	}
>  
> -	/* Teardown the gpadl on the vsp end */
> -	if (net_device->recv_buf_gpadl_handle) {
> -		ret = vmbus_teardown_gpadl(device->channel,
> -					   net_device->recv_buf_gpadl_handle);
> -
> -		/* If we failed here, we might as well return and have a leak
> -		 * rather than continue and a bugchk
> -		 */
> -		if (ret != 0) {
> -			netdev_err(ndev,
> -				   "unable to teardown receive buffer's gpadl\n");
> -			return;
> -		}
> -		net_device->recv_buf_gpadl_handle = 0;
> -	}
> -
> -	if (net_device->recv_buf) {
> -		/* Free up the receive buffer */
> -		vfree(net_device->recv_buf);
> -		net_device->recv_buf = NULL;
> -	}
> -
>  	/* Deal with the send buffer we may have setup.
>  	 * If we got a  send section size, it means we received a
>  	 * NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE msg (ie sent
> @@ -210,7 +187,35 @@ static void netvsc_destroy_buf(struct hv_device *device)
>  		}
>  		net_device->send_section_cnt = 0;
>  	}
> -	/* Teardown the gpadl on the vsp end */
> +}
> +
> +static void netvsc_teardown_gpadl(struct hv_device *device,
> +				  struct netvsc_device *net_device)
> +{
> +	struct net_device *ndev = hv_get_drvdata(device);
> +	int ret;
> +
> +	if (net_device->recv_buf_gpadl_handle) {
> +		ret = vmbus_teardown_gpadl(device->channel,
> +					   net_device->recv_buf_gpadl_handle);
> +
> +		/* If we failed here, we might as well return and have a leak
> +		 * rather than continue and a bugchk
> +		 */
> +		if (ret != 0) {
> +			netdev_err(ndev,
> +				   "unable to teardown receive buffer's gpadl\n");
> +			return;
> +		}
> +		net_device->recv_buf_gpadl_handle = 0;
> +	}
> +
> +	if (net_device->recv_buf) {
> +		/* Free up the receive buffer */
> +		vfree(net_device->recv_buf);
> +		net_device->recv_buf = NULL;
> +	}
> +
>  	if (net_device->send_buf_gpadl_handle) {
>  		ret = vmbus_teardown_gpadl(device->channel,
>  					   net_device->send_buf_gpadl_handle);
> @@ -420,7 +425,8 @@ static int netvsc_init_buf(struct hv_device *device,
>  	goto exit;
>  
>  cleanup:
> -	netvsc_destroy_buf(device);
> +	netvsc_revoke_buf(device, net_device);
> +	netvsc_teardown_gpadl(device, net_device);
>  
>  exit:
>  	return ret;
> @@ -539,11 +545,6 @@ static int netvsc_connect_vsp(struct hv_device *device,
>  	return ret;
>  }
>  
> -static void netvsc_disconnect_vsp(struct hv_device *device)
> -{
> -	netvsc_destroy_buf(device);
> -}
> -
>  /*
>   * netvsc_device_remove - Callback when the root bus device is removed
>   */
> @@ -557,7 +558,7 @@ void netvsc_device_remove(struct hv_device *device)
>  
>  	cancel_work_sync(&net_device->subchan_work);
>  
> -	netvsc_disconnect_vsp(device);
> +	netvsc_revoke_buf(device, net_device);
>  
>  	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
>  
> @@ -570,6 +571,8 @@ void netvsc_device_remove(struct hv_device *device)
>  	/* Now, we can close the channel safely */
>  	vmbus_close(device->channel);
>  
> +	netvsc_teardown_gpadl(device, net_device);
> +
>  	/* And dissassociate NAPI context from device */
>  	for (i = 0; i < net_device->num_chn; i++)
>  		netif_napi_del(&net_device->chan_table[i].napi);

This patch makes sense,
diff mbox series

Patch

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 5bb6a20072dd..bfc79698b8f4 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -100,12 +100,11 @@  static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
 	call_rcu(&nvdev->rcu, free_netvsc_device);
 }
 
-static void netvsc_destroy_buf(struct hv_device *device)
+static void netvsc_revoke_buf(struct hv_device *device,
+			      struct netvsc_device *net_device)
 {
 	struct nvsp_message *revoke_packet;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct net_device_context *ndc = netdev_priv(ndev);
-	struct netvsc_device *net_device = rtnl_dereference(ndc->nvdev);
 	int ret;
 
 	/*
@@ -148,28 +147,6 @@  static void netvsc_destroy_buf(struct hv_device *device)
 		net_device->recv_section_cnt = 0;
 	}
 
-	/* Teardown the gpadl on the vsp end */
-	if (net_device->recv_buf_gpadl_handle) {
-		ret = vmbus_teardown_gpadl(device->channel,
-					   net_device->recv_buf_gpadl_handle);
-
-		/* If we failed here, we might as well return and have a leak
-		 * rather than continue and a bugchk
-		 */
-		if (ret != 0) {
-			netdev_err(ndev,
-				   "unable to teardown receive buffer's gpadl\n");
-			return;
-		}
-		net_device->recv_buf_gpadl_handle = 0;
-	}
-
-	if (net_device->recv_buf) {
-		/* Free up the receive buffer */
-		vfree(net_device->recv_buf);
-		net_device->recv_buf = NULL;
-	}
-
 	/* Deal with the send buffer we may have setup.
 	 * If we got a  send section size, it means we received a
 	 * NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE msg (ie sent
@@ -210,7 +187,35 @@  static void netvsc_destroy_buf(struct hv_device *device)
 		}
 		net_device->send_section_cnt = 0;
 	}
-	/* Teardown the gpadl on the vsp end */
+}
+
+static void netvsc_teardown_gpadl(struct hv_device *device,
+				  struct netvsc_device *net_device)
+{
+	struct net_device *ndev = hv_get_drvdata(device);
+	int ret;
+
+	if (net_device->recv_buf_gpadl_handle) {
+		ret = vmbus_teardown_gpadl(device->channel,
+					   net_device->recv_buf_gpadl_handle);
+
+		/* If we failed here, we might as well return and have a leak
+		 * rather than continue and a bugchk
+		 */
+		if (ret != 0) {
+			netdev_err(ndev,
+				   "unable to teardown receive buffer's gpadl\n");
+			return;
+		}
+		net_device->recv_buf_gpadl_handle = 0;
+	}
+
+	if (net_device->recv_buf) {
+		/* Free up the receive buffer */
+		vfree(net_device->recv_buf);
+		net_device->recv_buf = NULL;
+	}
+
 	if (net_device->send_buf_gpadl_handle) {
 		ret = vmbus_teardown_gpadl(device->channel,
 					   net_device->send_buf_gpadl_handle);
@@ -420,7 +425,8 @@  static int netvsc_init_buf(struct hv_device *device,
 	goto exit;
 
 cleanup:
-	netvsc_destroy_buf(device);
+	netvsc_revoke_buf(device, net_device);
+	netvsc_teardown_gpadl(device, net_device);
 
 exit:
 	return ret;
@@ -539,11 +545,6 @@  static int netvsc_connect_vsp(struct hv_device *device,
 	return ret;
 }
 
-static void netvsc_disconnect_vsp(struct hv_device *device)
-{
-	netvsc_destroy_buf(device);
-}
-
 /*
  * netvsc_device_remove - Callback when the root bus device is removed
  */
@@ -557,7 +558,7 @@  void netvsc_device_remove(struct hv_device *device)
 
 	cancel_work_sync(&net_device->subchan_work);
 
-	netvsc_disconnect_vsp(device);
+	netvsc_revoke_buf(device, net_device);
 
 	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
 
@@ -570,6 +571,8 @@  void netvsc_device_remove(struct hv_device *device)
 	/* Now, we can close the channel safely */
 	vmbus_close(device->channel);
 
+	netvsc_teardown_gpadl(device, net_device);
+
 	/* And dissassociate NAPI context from device */
 	for (i = 0; i < net_device->num_chn; i++)
 		netif_napi_del(&net_device->chan_table[i].napi);