[{"id":1764240,"web_url":"http://patchwork.ozlabs.org/comment/1764240/","msgid":"","list_archive_url":null,"date":"2017-09-06T16:23:45","subject":"RE: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug","submitter":{"id":8234,"url":"http://patchwork.ozlabs.org/api/people/8234/","name":"Haiyang Zhang","email":"haiyangz@microsoft.com"},"content":"> -----Original Message-----\n> From: Stephen Hemminger [mailto:stephen@networkplumber.org]\n> Sent: Wednesday, September 6, 2017 11:19 AM\n> To: KY Srinivasan ; Haiyang Zhang\n> ; Stephen Hemminger \n> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org\n> Subject: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug\n> \n> When a virtual device is added dynamically (via host console), then\n> the vmbus sends an offer message for the primary channel. The processing\n> of this message for networking causes the network device to then\n> initialize the sub channels.\n> \n> The problem is that setting up the sub channels needs to wait until\n> the subsequent subchannel offers have been processed. These offers\n> come in on the same ring buffer and work queue as where the primary\n> offer is being processed; leading to a deadlock.\n> \n> This did not happen in older kernels, because the sub channel waiting\n> logic was broken (it wasn't really waiting).\n> \n> The solution is to do the sub channel setup in its own work queue\n> context that is scheduled by the primary channel setup; and then\n> happens later.\n> \n> Fixes: 732e49850c5e (\"netvsc: fix race on sub channel creation\")\n> Reported-by: Dexuan Cui \n> Signed-off-by: Stephen Hemminger \n> ---\n> Should also go to stable, but this version does not apply cleanly\n> to 4.13. Have another patch for that.\n> \n> drivers/net/hyperv/hyperv_net.h | 1 +\n> drivers/net/hyperv/netvsc_drv.c | 8 +--\n> drivers/net/hyperv/rndis_filter.c | 106 ++++++++++++++++++++++++++-----\n> -------\n> 3 files changed, 74 insertions(+), 41 deletions(-)\n\nThe patch looks overall. I just have a question:\n\nWith this patch, after module load and probe is done, there may still be\nsubchannels being processed. If rmmod immediately, the subchannel offers \nmay hit half-way removed device structures... Do we also need to add \ncancel_work_sync(&dev->subchan_work) to the top of netvsc_remove()?\n\nunregister_netdevice() includes device close, but it's only called later\nin the netvsc_remove() when rndis is already removed.\n\nThanks,\n- Haiyang","headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=microsoft.com header.i=@microsoft.com\n\theader.b=\"dFpZSyyt\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=haiyangz@microsoft.com; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnTRY6cWLz9s7F\n\tfor ;\n\tThu, 7 Sep 2017 02:23:53 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754832AbdIFQXv (ORCPT );\n\tWed, 6 Sep 2017 12:23:51 -0400","from mail-cys01nam02on0135.outbound.protection.outlook.com\n\t([104.47.37.135]:31904\n\t\"EHLO NAM02-CY1-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1754774AbdIFQXs (ORCPT );\n\tWed, 6 Sep 2017 12:23:48 -0400","from DM5PR21MB0475.namprd21.prod.outlook.com (10.172.92.17) by\n\tDM5PR21MB0764.namprd21.prod.outlook.com (10.173.172.22) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.0; Wed, 6 Sep 2017 16:23:46 +0000","from DM5PR21MB0475.namprd21.prod.outlook.com ([10.172.92.17]) by\n\tDM5PR21MB0475.namprd21.prod.outlook.com ([10.172.92.17]) with mapi id\n\t15.20.0056.003; Wed, 6 Sep 2017 16:23:45 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=bVbHt0DVWSwugHO6WvgmNJgiXFmhdhRjXSmx1Og6YuI=;\n\tb=dFpZSyytKBsyx9t5CfQhNEWbX4rgD8vzh/zXpN2wPuqsVf0aIJmpN71SihVuNxjRw+9pQTKW3RHx9vGPdxxIogHI6w8SlMIJPWMW7S2yqj1KhJrsNVeojWLlBGJJyROcXO2TkQAQ8BNDimNY0mpd1FP6gTsxONovBMtudlrhYnk=","From":"Haiyang Zhang ","To":"Stephen Hemminger ,\n\tKY Srinivasan ,\n\tStephen Hemminger ","CC":"\"devel@linuxdriverproject.org\" ,\n\t\"netdev@vger.kernel.org\" ","Subject":"RE: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug","Thread-Topic":"[PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug","Thread-Index":"AQHTJyOOddEQZyRqj0a43G82p46Wh6KoBivA","Date":"Wed, 6 Sep 2017 16:23:45 +0000","Message-ID":"","References":"<20170906151925.15221-1-sthemmin@microsoft.com>\n\t<20170906151925.15221-2-sthemmin@microsoft.com>","In-Reply-To":"<20170906151925.15221-2-sthemmin@microsoft.com>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","msip_labels":"MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Ref=https://api.informationprotection.azure.com/api/72f988bf-86f1-41af-91ab-2d7cd011db47;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Owner=haiyangz@microsoft.com;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2017-09-06T12:23:42.7095425-04:00;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=General;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Application=Microsoft\n\tAzure Information Protection;\n\tMSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Extended_MSFT_Method=Automatic;\n\tSensitivity=General","x-originating-ip":"[69.128.199.77]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; DM5PR21MB0764;\n\t6:9cjJIxrVRWq7BM3kSiDke6zeAmdg8XpLOj1Umk26dR+0gVrBqFMOr9vZnciJQp2EAwDJajAwl4TrIH1KC1141TD3Xu9Zo5VL6PYgYjBNzmKzLW67U6DOgJ70ISj/X9/snStHcoEeFLiUhP0yJl4R5ksufrXHVXGXGBXliTpJ86FAD/lrCobERJK8tYyhs/Hrpi/bMEmTy3Bn3JzeZ2hbm9s3Tar42D5Du0Gm8Hnlg3v10fK7oixtlxx9nfunQ/kRZv93WYQW9VRYTSA0fHuNlsdoSuEHYns/QsqQVuRZNGZFK3GmV0XH4iEmk0gHsdPbxJhOgOKqo0AsFxQDp3u2IA==;\n\t5:FLYrl9GIk4hbKYu87jTZZun2XeFplyBznD7djjemOzvDd762UR2a0M7OPZ0VQ3Ermgf82NxDFlNOV8Nku0ehkmXawc508be0iKsINJA+fJhUx8rERsEHmdU5xhc8W4bvsgWPr2nmVz5SFWyINWh5ew==;\n\t24:GyW2pk81WhnXxZtaZOofP+dPp4g+m7yvEG+yXCkNioqH2wB1kkQJOfJm8ShcVNsmy8KVBZjI95TyMVy+c1auwj4YGA4+FoQ3raC7P5xXQmE=;\n\t7:oXNUeM46OyTM7ZKh3JJ2LoSFP8cp8iVQQ2X8iyekuvG8hKWJGEZHOyhVC86Qy/8axRocG4HmabpgKiVdBQ0kG2Mza/1L2DaIpsUUZqKIyRptMMVG19t2DgdqshFXYIGqLxQNTuhbXZJnTNkvxX6XiLQKFfIZnGjURBI4iUmzRAh6krsuJc7iHDCDHqGCKqnrCAY2Fg8redgGMZf6lJKUmctOExXGrOVFW2w8J3aBuaU=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;SSOR;","x-forefront-antispam-report":"SFV:SKI; SCL:-1; SFV:NSPM;\n\tSFS:(10019020)(6009001)(39860400002)(47760400005)(189002)(377454003)(13464003)(199003)(66066001)(7696004)(6506006)(55016002)(54906002)(53936002)(99286003)(74316002)(5005710100001)(8990500004)(10290500003)(4326008)(2561002)(2950100002)(8936002)(86362001)(53546010)(25786009)(86612001)(10090500001)(6636002)(1511001)(101416001)(478600001)(106356001)(77096006)(3280700002)(105586002)(3660700001)(81166006)(81156014)(8676002)(229853002)(2906002)(50986999)(76176999)(54356999)(33656002)(14454004)(7736002)(6116002)(102836003)(3846002)(189998001)(9686003)(6436002)(2900100001)(2421001)(68736007)(5660300001)(6246003)(305945005)(97736004);\n\tDIR:OUT; SFP:1102; SCL:1; SRVR:DM5PR21MB0764;\n\tH:DM5PR21MB0475.namprd21.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","x-ms-office365-filtering-correlation-id":"7e5e7acb-fad3-485f-7db2-08d4f543a5b4","x-ms-office365-filtering-ht":"Tenant","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(48565401081)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:DM5PR21MB0764; ","x-ms-traffictypediagnostic":"DM5PR21MB0764:","x-exchange-antispam-report-test":"UriScan:(89211679590171)(9452136761055);","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(61425038)(6040450)(2401047)(8121501046)(5005006)(3002001)(10201501046)(100000703101)(100105400095)(93006095)(93001095)(6055026)(61426038)(61427038)(6041248)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(20161123558100)(20161123564025)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:DM5PR21MB0764; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:DM5PR21MB0764; ","x-forefront-prvs":"0422860ED4","received-spf":"None (protection.outlook.com: microsoft.com does not designate\n\tpermitted sender hosts)","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=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=microsoft.com header.i=@microsoft.com\n\theader.b=\"dFpZSyyt\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=haiyangz@microsoft.com; "],"spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"quoted-printable","MIME-Version":"1.0","X-OriginatorOrg":"microsoft.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"06 Sep 2017 16:23:45.6047\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"72f988bf-86f1-41af-91ab-2d7cd011db47","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DM5PR21MB0764","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1764251,"web_url":"http://patchwork.ozlabs.org/comment/1764251/","msgid":"<20170906093638.2074f455@xeon-e3>","list_archive_url":null,"date":"2017-09-06T16:36:38","subject":"Re: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug","submitter":{"id":21389,"url":"http://patchwork.ozlabs.org/api/people/21389/","name":"Stephen Hemminger","email":"stephen@networkplumber.org"},"content":"On Wed, 6 Sep 2017 16:23:45 +0000\nHaiyang Zhang wrote:\n\n> > -----Original Message-----\n> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]\n> > Sent: Wednesday, September 6, 2017 11:19 AM\n> > To: KY Srinivasan ; Haiyang Zhang\n> > ; Stephen Hemminger \n> > Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org\n> > Subject: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug\n> > \n> > When a virtual device is added dynamically (via host console), then\n> > the vmbus sends an offer message for the primary channel. The processing\n> > of this message for networking causes the network device to then\n> > initialize the sub channels.\n> > \n> > The problem is that setting up the sub channels needs to wait until\n> > the subsequent subchannel offers have been processed. These offers\n> > come in on the same ring buffer and work queue as where the primary\n> > offer is being processed; leading to a deadlock.\n> > \n> > This did not happen in older kernels, because the sub channel waiting\n> > logic was broken (it wasn't really waiting).\n> > \n> > The solution is to do the sub channel setup in its own work queue\n> > context that is scheduled by the primary channel setup; and then\n> > happens later.\n> > \n> > Fixes: 732e49850c5e (\"netvsc: fix race on sub channel creation\")\n> > Reported-by: Dexuan Cui \n> > Signed-off-by: Stephen Hemminger \n> > ---\n> > Should also go to stable, but this version does not apply cleanly\n> > to 4.13. Have another patch for that.\n> > \n> > drivers/net/hyperv/hyperv_net.h | 1 +\n> > drivers/net/hyperv/netvsc_drv.c | 8 +--\n> > drivers/net/hyperv/rndis_filter.c | 106 ++++++++++++++++++++++++++-----\n> > -------\n> > 3 files changed, 74 insertions(+), 41 deletions(-) \n> \n> The patch looks overall. I just have a question:\n> \n> With this patch, after module load and probe is done, there may still be\n> subchannels being processed. If rmmod immediately, the subchannel offers \n> may hit half-way removed device structures... Do we also need to add \n> cancel_work_sync(&dev->subchan_work) to the top of netvsc_remove()?\n> \n> unregister_netdevice() includes device close, but it's only called later\n> in the netvsc_remove() when rndis is already removed.\n> \n> Thanks,\n> - Haiyang\n\nGood catch.\nIf the driver called unregister_netdevice first before doing rndis_filter_device_remove\nthat would solve the problem. That wouldn't cause additional problems and it makes\nsense to close the network layer first.","headers":{"Return-Path":"","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=)","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=\"WwMAqnk+\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnTkt1nkjz9sRV\n\tfor ;\n\tThu, 7 Sep 2017 02:37:10 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755484AbdIFQgs (ORCPT );\n\tWed, 6 Sep 2017 12:36:48 -0400","from mail-pg0-f43.google.com ([74.125.83.43]:34108 \"EHLO\n\tmail-pg0-f43.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753890AbdIFQgp (ORCPT\n\t); Wed, 6 Sep 2017 12:36:45 -0400","by mail-pg0-f43.google.com with SMTP id q68so295883pgq.1\n\tfor ; Wed, 06 Sep 2017 09:36:45 -0700 (PDT)","from xeon-e3 (76-14-207-240.or.wavecable.com. [76.14.207.240])\n\tby smtp.gmail.com with ESMTPSA id\n\tt84sm321543pfa.149.2017.09.06.09.36.45\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 06 Sep 2017 09:36:45 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=networkplumber-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=ejmNrV3XEugUMvuNttwr6FU5s0NZgdFRleJOdxAHa8U=;\n\tb=WwMAqnk+9I5RO/qFMK1v4gkkROkcKLqloQBs3zDkR1zBI25hb4Y5LfwQs4PtDqqSun\n\tyElDYOsxdXZkXjpP+CahYyw9wdIf4LCNSfNggf38s+yojJo3+mq87BsvbeOlUykiUjij\n\t9DUzsUayTNARJLzNf6zeXGUrYjodR2mc5TFIW2ue6DmaI9XtUWXeqPW1VEaail1J1oKh\n\tXDusIDnqBrVv0Vn294O6AxEzQgDnm1NnpdP/XQSrH+7xqpCikkdiAu8DO4WMf9Tw9k3U\n\tvABjop27GbeJbAjVafZzh6ii4Q8TSrEXFZkfJzbVHB+OdhjxHA5yMApFn7Zz8Mc1NsHY\n\tTwOw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=ejmNrV3XEugUMvuNttwr6FU5s0NZgdFRleJOdxAHa8U=;\n\tb=nHJHwfAXaaaRkLUM/4wQEOEONjpi0rME5cOTi3yEX4BFT1Peh2S91HzpkaN1FYO9u/\n\tCFWP4EbVq0KYdJ2YwEyWAnQ/NUYXrxr8IZNEguNNgBYckLHAyDFsF39u1WIy5m/B5Vlk\n\tWYivF4Ipcq9R/BrH7/d2c8vnT5NDhNMMUJ24CM69jitOzO/YfESD6g6q/Bk4EXINGT4V\n\trRSYQA2EcJhnSpN7tvaG369nn42/9/gmE4Kj9Rdh4S02xs3fV87CxA1Uqr1GajQtvaVu\n\tc3zEQbUczpuziOkZKQvq0RJYO+PU3VFjWUvnggh6HB0N1+5YHx35tT/iw55/pSTN1hBt\n\t3nrA==","X-Gm-Message-State":"AHPjjUgzOK2hEZNkhSyC6g6VZMPa+iRNKFUPyCYyZEKnOH+DUQC83lGR\n\tTDmWwwa6jQXiZl/V","X-Google-Smtp-Source":"ADKCNb5f+Cn9FqowlXpDfF4Gy9pHeLRr35C4/qnw5HKA8Zi4b4eChINhGRR4NKSQp2NQfFR3Ha3Nrw==","X-Received":"by 10.98.89.151 with SMTP id k23mr8061449pfj.69.1504715805483;\n\tWed, 06 Sep 2017 09:36:45 -0700 (PDT)","Date":"Wed, 6 Sep 2017 09:36:38 -0700","From":"Stephen Hemminger ","To":"Haiyang Zhang ","Cc":"KY Srinivasan ,\n\tStephen Hemminger ,\n\t\"devel@linuxdriverproject.org\" ,\n\t\"netdev@vger.kernel.org\" ","Subject":"Re: [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug","Message-ID":"<20170906093638.2074f455@xeon-e3>","In-Reply-To":"","References":"<20170906151925.15221-1-sthemmin@microsoft.com>\n\t<20170906151925.15221-2-sthemmin@microsoft.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]