Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/810794/?format=api
{ "id": 810794, "url": "http://patchwork.ozlabs.org/api/patches/810794/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/20170906205306.10541-2-sthemmin@microsoft.com/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170906205306.10541-2-sthemmin@microsoft.com>", "list_archive_url": null, "date": "2017-09-06T20:53:05", "name": "[v2,net-next,1/2] hv_netvsc: fix deadlock on hotplug", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "e683450253370a63fd4d2805496a4f54065c9134", "submitter": { "id": 21389, "url": "http://patchwork.ozlabs.org/api/people/21389/?format=api", "name": "Stephen Hemminger", "email": "stephen@networkplumber.org" }, "delegate": { "id": 34, "url": "http://patchwork.ozlabs.org/api/users/34/?format=api", "username": "davem", "first_name": "David", "last_name": "Miller", "email": "davem@davemloft.net" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/20170906205306.10541-2-sthemmin@microsoft.com/mbox/", "series": [ { "id": 1878, "url": "http://patchwork.ozlabs.org/api/series/1878/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/list/?series=1878", "date": "2017-09-06T20:53:06", "name": "hv_netvsc: sub channel initialization fixes", "version": 2, "mbox": "http://patchwork.ozlabs.org/series/1878/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/810794/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/810794/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": [ "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=networkplumber-org.20150623.gappssmtp.com\n\theader.i=@networkplumber-org.20150623.gappssmtp.com\n\theader.b=\"ujSu1x35\"; dkim-atps=neutral" ], "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnbQv1l5Kz9ryQ\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 7 Sep 2017 06:53:43 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753303AbdIFUxl (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 6 Sep 2017 16:53:41 -0400", "from mail-pg0-f49.google.com ([74.125.83.49]:34207 \"EHLO\n\tmail-pg0-f49.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753017AbdIFUxR (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 6 Sep 2017 16:53:17 -0400", "by mail-pg0-f49.google.com with SMTP id q68so1384185pgq.1\n\tfor <netdev@vger.kernel.org>; Wed, 06 Sep 2017 13:53:16 -0700 (PDT)", "from xeon-e3.lan (76-14-207-240.or.wavecable.com. [76.14.207.240])\n\tby smtp.gmail.com with ESMTPSA id\n\tx130sm841171pfd.65.2017.09.06.13.53.15\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 06 Sep 2017 13:53:15 -0700 (PDT)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=networkplumber-org.20150623.gappssmtp.com; s=20150623;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=Pnc2+oSCEWjoemSGt0BTJMljOds9uVBJ6TEZKSGjlU8=;\n\tb=ujSu1x356bl6F2J6sgBgh9iXSCsDoTJK0Z2VpQ17xWYjKNzI04btnFSmvYKt+pGiFa\n\tKeY+bR6ZUKS8ejIxqgmmiG+dPRjdZrUt8ziKhFN9G+TsL77Wpk4og1iH7UQDhjsFWcgm\n\t5E/RJ3h7oAOtiJWHxNbqPx/WiFKybd6hcHvkVKjRKVgsruaV8IloY3vIzdEM+UIFCiDr\n\tuuEl4s578VfksYJPXHHUqZyiZSPfciJ1jaTmaduoWBVeD0vQ88NfD1kx1IuVpuxfAD6q\n\tNbxb6hVG9mVm8tqvrvCOavCrBZ+PLza97jroJ6iJx08eAHrP5TkLDT07cw4m75ulpheI\n\t3SIw==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references;\n\tbh=Pnc2+oSCEWjoemSGt0BTJMljOds9uVBJ6TEZKSGjlU8=;\n\tb=ARQNrU4nX/qSKYbVI57OL1AjbDsP8jcrQyg//SRolTedLfNCDHjHH4mnSq4fSCwsb3\n\tLiee67z/5Q9xy4YJKM3tQFid9HeE7Fsosc5t0kB3WkMzQQEzFXWux9D6jJar/aqhMvGc\n\tCpV5GsIMLhQTa0sq77IFYVLWiBJb7oTUJW2P9C0gf+jv8pZ5gkvant9PeFi/D5x7DvPB\n\tkve3EXn4qmXkwqd91fXyRHfyq/QcXZFf1jnWPdEvIhmHDOe3PPPK2NHzcp3It/66CyF/\n\tSiwzx1Vd9DhsTbkBJlP4axKieuF70W3cgJQf7HBB0TmaaiBaDEWkOLpz3my7PfuIPvOX\n\t/XaQ==", "X-Gm-Message-State": "AHPjjUj1dMbOvq89JwvuP3/ON/CvUcCGiaLJStUGTXWvlwtFxeaimmGh\n\tblHUjLuGtIMS+ezzEcmDxg==", "X-Google-Smtp-Source": "ADKCNb6DMZnXsRlHyBFTcFlq88awzLTnY4aE68YqPckJtnmsAmSuWVi2prteg1+PWmbZJHzGl9YVPw==", "X-Received": "by 10.98.33.25 with SMTP id h25mr397744pfh.209.1504731196447;\n\tWed, 06 Sep 2017 13:53:16 -0700 (PDT)", "From": "Stephen Hemminger <stephen@networkplumber.org>", "X-Google-Original-From": "Stephen Hemminger <sthemmin@microsoft.com>", "To": "kys@microsoft.com, haiyangz@microsoft.com, sthemmin@microsoft.com", "Cc": "devel@linuxdriverproject.org, netdev@vger.kernel.org", "Subject": "[PATCH v2 net-next 1/2] hv_netvsc: fix deadlock on hotplug", "Date": "Wed, 6 Sep 2017 13:53:05 -0700", "Message-Id": "<20170906205306.10541-2-sthemmin@microsoft.com>", "X-Mailer": "git-send-email 2.11.0", "In-Reply-To": "<20170906205306.10541-1-sthemmin@microsoft.com>", "References": "<20170906205306.10541-1-sthemmin@microsoft.com>", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "When a virtual device is added dynamically (via host console), then\nthe vmbus sends an offer message for the primary channel. The processing\nof this message for networking causes the network device to then\ninitialize the sub channels.\n\nThe problem is that setting up the sub channels needs to wait until\nthe subsequent subchannel offers have been processed. These offers\ncome in on the same ring buffer and work queue as where the primary\noffer is being processed; leading to a deadlock.\n\nThis did not happen in older kernels, because the sub channel waiting\nlogic was broken (it wasn't really waiting).\n\nThe solution is to do the sub channel setup in its own work queue\ncontext that is scheduled by the primary channel setup; and then\nhappens later.\n\nFixes: 732e49850c5e (\"netvsc: fix race on sub channel creation\")\nReported-by: Dexuan Cui <decui@microsoft.com>\nSigned-off-by: Stephen Hemminger <sthemmin@microsoft.com>\n---\nv2 - fix module removal race with new work queue\n\n drivers/net/hyperv/hyperv_net.h | 3 +\n drivers/net/hyperv/netvsc.c | 3 +\n drivers/net/hyperv/netvsc_drv.c | 11 +---\n drivers/net/hyperv/rndis_filter.c | 122 ++++++++++++++++++++++++++------------\n 4 files changed, 94 insertions(+), 45 deletions(-)", "diff": "diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h\nindex ec546da86683..d98cdfb1536b 100644\n--- a/drivers/net/hyperv/hyperv_net.h\n+++ b/drivers/net/hyperv/hyperv_net.h\n@@ -204,6 +204,8 @@ int netvsc_recv_callback(struct net_device *net,\n \t\t\t const struct ndis_pkt_8021q_info *vlan);\n void netvsc_channel_cb(void *context);\n int netvsc_poll(struct napi_struct *napi, int budget);\n+\n+void rndis_set_subchannel(struct work_struct *w);\n bool rndis_filter_opened(const struct netvsc_device *nvdev);\n int rndis_filter_open(struct netvsc_device *nvdev);\n int rndis_filter_close(struct netvsc_device *nvdev);\n@@ -782,6 +784,7 @@ struct netvsc_device {\n \tu32 num_chn;\n \n \tatomic_t open_chn;\n+\tstruct work_struct subchan_work;\n \twait_queue_head_t subchan_open;\n \n \tstruct rndis_device *extension;\ndiff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c\nindex 0062b802676f..a5511b7326af 100644\n--- a/drivers/net/hyperv/netvsc.c\n+++ b/drivers/net/hyperv/netvsc.c\n@@ -81,6 +81,7 @@ static struct netvsc_device *alloc_net_device(void)\n \n \tinit_completion(&net_device->channel_init_wait);\n \tinit_waitqueue_head(&net_device->subchan_open);\n+\tINIT_WORK(&net_device->subchan_work, rndis_set_subchannel);\n \n \treturn net_device;\n }\n@@ -557,6 +558,8 @@ void netvsc_device_remove(struct hv_device *device)\n \t\t= rtnl_dereference(net_device_ctx->nvdev);\n \tint i;\n \n+\tcancel_work_sync(&net_device->subchan_work);\n+\n \tnetvsc_disconnect_vsp(device);\n \n \tRCU_INIT_POINTER(net_device_ctx->nvdev, NULL);\ndiff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c\nindex 165ba4b3b423..c538a4f15f3b 100644\n--- a/drivers/net/hyperv/netvsc_drv.c\n+++ b/drivers/net/hyperv/netvsc_drv.c\n@@ -853,10 +853,7 @@ static int netvsc_set_channels(struct net_device *net,\n \trndis_filter_device_remove(dev, nvdev);\n \n \tnvdev = rndis_filter_device_add(dev, &device_info);\n-\tif (!IS_ERR(nvdev)) {\n-\t\tnetif_set_real_num_tx_queues(net, nvdev->num_chn);\n-\t\tnetif_set_real_num_rx_queues(net, nvdev->num_chn);\n-\t} else {\n+\tif (IS_ERR(nvdev)) {\n \t\tret = PTR_ERR(nvdev);\n \t\tdevice_info.num_chn = orig;\n \t\tnvdev = rndis_filter_device_add(dev, &device_info);\n@@ -1954,9 +1951,6 @@ static int netvsc_probe(struct hv_device *dev,\n \t\tNETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;\n \tnet->vlan_features = net->features;\n \n-\tnetif_set_real_num_tx_queues(net, nvdev->num_chn);\n-\tnetif_set_real_num_rx_queues(net, nvdev->num_chn);\n-\n \tnetdev_lockdep_set_classes(net);\n \n \t/* MTU range: 68 - 1500 or 65521 */\n@@ -2012,9 +2006,10 @@ static int netvsc_remove(struct hv_device *dev)\n \tif (vf_netdev)\n \t\tnetvsc_unregister_vf(vf_netdev);\n \n+\tunregister_netdevice(net);\n+\n \trndis_filter_device_remove(dev,\n \t\t\t\t rtnl_dereference(ndev_ctx->nvdev));\n-\tunregister_netdevice(net);\n \trtnl_unlock();\n \n \thv_set_drvdata(dev, NULL);\ndiff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c\nindex 69c40b8fccc3..731bc7cc6f43 100644\n--- a/drivers/net/hyperv/rndis_filter.c\n+++ b/drivers/net/hyperv/rndis_filter.c\n@@ -1039,8 +1039,6 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)\n \n \t/* Set the channel before opening.*/\n \tnvchan->channel = new_sc;\n-\tnetif_napi_add(ndev, &nvchan->napi,\n-\t\t netvsc_poll, NAPI_POLL_WEIGHT);\n \n \tret = vmbus_open(new_sc, nvscdev->ring_size * PAGE_SIZE,\n \t\t\t nvscdev->ring_size * PAGE_SIZE, NULL, 0,\n@@ -1048,12 +1046,88 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)\n \tif (ret == 0)\n \t\tnapi_enable(&nvchan->napi);\n \telse\n-\t\tnetif_napi_del(&nvchan->napi);\n+\t\tnetdev_notice(ndev, \"sub channel open failed: %d\\n\", ret);\n \n \tatomic_inc(&nvscdev->open_chn);\n \twake_up(&nvscdev->subchan_open);\n }\n \n+/* Open sub-channels after completing the handling of the device probe.\n+ * This breaks overlap of processing the host message for the\n+ * new primary channel with the initialization of sub-channels.\n+ */\n+void rndis_set_subchannel(struct work_struct *w)\n+{\n+\tstruct netvsc_device *nvdev\n+\t\t= container_of(w, struct netvsc_device, subchan_work);\n+\tstruct nvsp_message *init_packet = &nvdev->channel_init_pkt;\n+\tstruct net_device_context *ndev_ctx;\n+\tstruct rndis_device *rdev;\n+\tstruct net_device *ndev;\n+\tstruct hv_device *hv_dev;\n+\tint i, ret;\n+\n+\tif (!rtnl_trylock()) {\n+\t\tschedule_work(w);\n+\t\treturn;\n+\t}\n+\n+\trdev = nvdev->extension;\n+\tif (!rdev)\n+\t\tgoto unlock;\t/* device was removed */\n+\n+\tndev = rdev->ndev;\n+\tndev_ctx = netdev_priv(ndev);\n+\thv_dev = ndev_ctx->device_ctx;\n+\n+\tmemset(init_packet, 0, sizeof(struct nvsp_message));\n+\tinit_packet->hdr.msg_type = NVSP_MSG5_TYPE_SUBCHANNEL;\n+\tinit_packet->msg.v5_msg.subchn_req.op = NVSP_SUBCHANNEL_ALLOCATE;\n+\tinit_packet->msg.v5_msg.subchn_req.num_subchannels =\n+\t\t\t\t\t\tnvdev->num_chn - 1;\n+\tret = vmbus_sendpacket(hv_dev->channel, init_packet,\n+\t\t\t sizeof(struct nvsp_message),\n+\t\t\t (unsigned long)init_packet,\n+\t\t\t VM_PKT_DATA_INBAND,\n+\t\t\t VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);\n+\tif (ret) {\n+\t\tnetdev_err(ndev, \"sub channel allocate send failed: %d\\n\", ret);\n+\t\tgoto failed;\n+\t}\n+\n+\twait_for_completion(&nvdev->channel_init_wait);\n+\tif (init_packet->msg.v5_msg.subchn_comp.status != NVSP_STAT_SUCCESS) {\n+\t\tnetdev_err(ndev, \"sub channel request failed\\n\");\n+\t\tgoto failed;\n+\t}\n+\n+\tnvdev->num_chn = 1 +\n+\t\tinit_packet->msg.v5_msg.subchn_comp.num_subchannels;\n+\n+\t/* wait for all sub channels to open */\n+\twait_event(nvdev->subchan_open,\n+\t\t atomic_read(&nvdev->open_chn) == nvdev->num_chn);\n+\n+\t/* ignore failues from setting rss parameters, still have channels */\n+\trndis_filter_set_rss_param(rdev, netvsc_hash_key);\n+\n+\tnetif_set_real_num_tx_queues(ndev, nvdev->num_chn);\n+\tnetif_set_real_num_rx_queues(ndev, nvdev->num_chn);\n+\n+\trtnl_unlock();\n+\treturn;\n+\n+failed:\n+\t/* fallback to only primary channel */\n+\tfor (i = 1; i < nvdev->num_chn; i++)\n+\t\tnetif_napi_del(&nvdev->chan_table[i].napi);\n+\n+\tnvdev->max_chn = 1;\n+\tnvdev->num_chn = 1;\n+unlock:\n+\trtnl_unlock();\n+}\n+\n struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,\n \t\t\t\t struct netvsc_device_info *device_info)\n {\n@@ -1063,7 +1137,6 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,\n \tstruct rndis_device *rndis_device;\n \tstruct ndis_offload hwcaps;\n \tstruct ndis_offload_params offloads;\n-\tstruct nvsp_message *init_packet;\n \tstruct ndis_recv_scale_cap rsscap;\n \tu32 rsscap_size = sizeof(struct ndis_recv_scale_cap);\n \tunsigned int gso_max_size = GSO_MAX_SIZE;\n@@ -1215,9 +1288,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,\n \t\t\t\t\t\t\tnet_device->num_chn);\n \n \tatomic_set(&net_device->open_chn, 1);\n-\n-\tif (net_device->num_chn == 1)\n-\t\treturn net_device;\n+\tvmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);\n \n \tfor (i = 1; i < net_device->num_chn; i++) {\n \t\tret = netvsc_alloc_recv_comp_ring(net_device, i);\n@@ -1228,38 +1299,15 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,\n \t\t}\n \t}\n \n-\tvmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);\n+\tfor (i = 1; i < net_device->num_chn; i++)\n+\t\tnetif_napi_add(net, &net_device->chan_table[i].napi,\n+\t\t\t netvsc_poll, NAPI_POLL_WEIGHT);\n \n-\tinit_packet = &net_device->channel_init_pkt;\n-\tmemset(init_packet, 0, sizeof(struct nvsp_message));\n-\tinit_packet->hdr.msg_type = NVSP_MSG5_TYPE_SUBCHANNEL;\n-\tinit_packet->msg.v5_msg.subchn_req.op = NVSP_SUBCHANNEL_ALLOCATE;\n-\tinit_packet->msg.v5_msg.subchn_req.num_subchannels =\n-\t\t\t\t\t\tnet_device->num_chn - 1;\n-\tret = vmbus_sendpacket(dev->channel, init_packet,\n-\t\t\t sizeof(struct nvsp_message),\n-\t\t\t (unsigned long)init_packet,\n-\t\t\t VM_PKT_DATA_INBAND,\n-\t\t\t VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);\n-\tif (ret)\n-\t\tgoto out;\n-\n-\twait_for_completion(&net_device->channel_init_wait);\n-\tif (init_packet->msg.v5_msg.subchn_comp.status != NVSP_STAT_SUCCESS) {\n-\t\tret = -ENODEV;\n-\t\tgoto out;\n-\t}\n+\tif (net_device->num_chn > 1)\n+\t\tschedule_work(&net_device->subchan_work);\n \n-\tnet_device->num_chn = 1 +\n-\t\tinit_packet->msg.v5_msg.subchn_comp.num_subchannels;\n-\n-\t/* wait for all sub channels to open */\n-\twait_event(net_device->subchan_open,\n-\t\t atomic_read(&net_device->open_chn) == net_device->num_chn);\n-\n-\t/* ignore failues from setting rss parameters, still have channels */\n-\trndis_filter_set_rss_param(rndis_device, netvsc_hash_key);\n out:\n+\t/* if unavailable, just proceed with one queue */\n \tif (ret) {\n \t\tnet_device->max_chn = 1;\n \t\tnet_device->num_chn = 1;\n@@ -1280,10 +1328,10 @@ void rndis_filter_device_remove(struct hv_device *dev,\n \t/* Halt and release the rndis device */\n \trndis_filter_halt_device(rndis_dev);\n \n-\tkfree(rndis_dev);\n \tnet_dev->extension = NULL;\n \n \tnetvsc_device_remove(dev);\n+\tkfree(rndis_dev);\n }\n \n int rndis_filter_open(struct netvsc_device *nvdev)\n", "prefixes": [ "v2", "net-next", "1/2" ] }