From patchwork Thu Mar 9 19:41:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Igor Druzhinin X-Patchwork-Id: 9614103 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 507AE60417 for ; Thu, 9 Mar 2017 19:44:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 392952869F for ; Thu, 9 Mar 2017 19:44:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2D98C286A7; Thu, 9 Mar 2017 19:44:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 717DC2869F for ; Thu, 9 Mar 2017 19:44:21 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cm3wa-0002Xk-2l; Thu, 09 Mar 2017 19:41:48 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cm3wZ-0002Xd-6S for xen-devel@lists.xenproject.org; Thu, 09 Mar 2017 19:41:47 +0000 Received: from [85.158.139.211] by server-5.bemta-5.messagelabs.com id C1/85-29481-AFFA1C85; Thu, 09 Mar 2017 19:41:46 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrOLMWRWlGSWpSXmKPExsXitHRDpO7P9Qc jDBp2a1l83zKZyYHR4/CHKywBjFGsmXlJ+RUJrBkTbi5lK1ijVjH5jUUD43SFLkZODgkBf4kV 5zaygNhsAgYSpzYtArNFBOwkLm0+wA5iMwuUSLSdXM8KYgsL+EjM2HIUqIaDg0VAReLBjkCQM K+Ah8T7+6dYIEYqSEx5+J4ZxBYSUJM42rWLBaJGUOLkzCcsECMlJA6+eME8gZF7FpLULCSpBY xMqxg1ilOLylKLdA3N9ZKKMtMzSnITM3N0DQ1M9XJTi4sT01NzEpOK9ZLzczcxAgOBAQh2MF4 87XmIUZKDSUmU9/PcgxFCfEn5KZUZicUZ8UWlOanFhxhlODiUJHgd1wHlBItS01Mr0jJzgCEJ k5bg4FES4Q0ESfMWFyTmFmemQ6ROMepyzJm9+w2TEEtefl6qlDjv5rVARQIgRRmleXAjYPFxi VFWSpiXEegoIZ6C1KLczBJU+VeM4hyMSsK8QSCreDLzSuA2vQI6ggnoiGl8YEeUJCKkpBoY5d mNtAvnVE0O+BWYZmtq0/rySY4sm6bj3Morzd/yzl1LCVO5X1K14XvDx+lzb57dpMKzyqDDPWr 1+ieHPgnNNzQV2xt0/Y+YgaK1U/X6wBVLQzZl/Vl91eCI692NAiJZvAz/srfY1Ufwa38vebHw oLy27vyWzFl1kYFzW+awV3TXP9ygEcKvxFKckWioxVxUnAgA0GWTW4oCAAA= X-Env-Sender: prvs=234c6cd0c=igor.druzhinin@citrix.com X-Msg-Ref: server-3.tower-206.messagelabs.com!1489088503!85107601!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 9.2.3; banners=-,-,- X-VirusChecked: Checked Received: (qmail 15746 invoked from network); 9 Mar 2017 19:41:45 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-3.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 9 Mar 2017 19:41:45 -0000 X-IronPort-AV: E=Sophos;i="5.36,137,1486425600"; d="scan'208";a="412754258" From: Igor Druzhinin To: , Date: Thu, 9 Mar 2017 19:41:37 +0000 Message-ID: <1489088497-9027-1-git-send-email-igor.druzhinin@citrix.com> X-Mailer: git-send-email 1.8.3.1 MIME-Version: 1.0 Cc: jgross@suse.com, Igor Druzhinin , paul.durrant@citrix.com, wei.liu2@citrix.com Subject: [Xen-devel] [PATCH net v3] xen-netback: fix race condition on XenBus disconnect X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP In some cases during XenBus disconnect event handling and subsequent queue resource release there may be some TX handlers active on other processors. Use RCU in order to synchronize with them. Signed-off-by: Igor Druzhinin Reviewed-by: Paul Durrant --- v3: * Fix unintended semantic change in xenvif_get_ethtool_stats * Dropped extra code v2: * Add protection for xenvif_get_ethtool_stats * Additional comments and fixes --- drivers/net/xen-netback/interface.c | 26 +++++++++++++++++--------- drivers/net/xen-netback/netback.c | 2 +- drivers/net/xen-netback/xenbus.c | 20 ++++++++++---------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index 829b26c..a3c018e 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -165,13 +165,17 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct xenvif *vif = netdev_priv(dev); struct xenvif_queue *queue = NULL; - unsigned int num_queues = vif->num_queues; + unsigned int num_queues; u16 index; struct xenvif_rx_cb *cb; BUG_ON(skb->dev != dev); - /* Drop the packet if queues are not set up */ + /* Drop the packet if queues are not set up. + * This handler should be called inside an RCU read section + * so we don't need to enter it here explicitly. + */ + num_queues = rcu_dereference(vif)->num_queues; if (num_queues < 1) goto drop; @@ -222,18 +226,18 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev) { struct xenvif *vif = netdev_priv(dev); struct xenvif_queue *queue = NULL; + unsigned int num_queues; u64 rx_bytes = 0; u64 rx_packets = 0; u64 tx_bytes = 0; u64 tx_packets = 0; unsigned int index; - spin_lock(&vif->lock); - if (vif->queues == NULL) - goto out; + rcu_read_lock(); + num_queues = rcu_dereference(vif)->num_queues; /* Aggregate tx and rx stats from each queue */ - for (index = 0; index < vif->num_queues; ++index) { + for (index = 0; index < num_queues; ++index) { queue = &vif->queues[index]; rx_bytes += queue->stats.rx_bytes; rx_packets += queue->stats.rx_packets; @@ -241,8 +245,7 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev) tx_packets += queue->stats.tx_packets; } -out: - spin_unlock(&vif->lock); + rcu_read_unlock(); vif->dev->stats.rx_bytes = rx_bytes; vif->dev->stats.rx_packets = rx_packets; @@ -378,10 +381,13 @@ static void xenvif_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *stats, u64 * data) { struct xenvif *vif = netdev_priv(dev); - unsigned int num_queues = vif->num_queues; + unsigned int num_queues; int i; unsigned int queue_index; + rcu_read_lock(); + num_queues = rcu_dereference(vif)->num_queues; + for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++) { unsigned long accum = 0; for (queue_index = 0; queue_index < num_queues; ++queue_index) { @@ -390,6 +396,8 @@ static void xenvif_get_ethtool_stats(struct net_device *dev, } data[i] = accum; } + + rcu_read_unlock(); } static void xenvif_get_strings(struct net_device *dev, u32 stringset, u8 * data) diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index f9bcf4a..602d408 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -214,7 +214,7 @@ static void xenvif_fatal_tx_err(struct xenvif *vif) netdev_err(vif->dev, "fatal error; disabling device\n"); vif->disabled = true; /* Disable the vif from queue 0's kthread */ - if (vif->queues) + if (vif->num_queues) xenvif_kick_thread(&vif->queues[0]); } diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index d2d7cd9..a56d3ea 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -495,26 +495,26 @@ static void backend_disconnect(struct backend_info *be) struct xenvif *vif = be->vif; if (vif) { + unsigned int num_queues = vif->num_queues; unsigned int queue_index; - struct xenvif_queue *queues; xen_unregister_watchers(vif); #ifdef CONFIG_DEBUG_FS xenvif_debugfs_delif(vif); #endif /* CONFIG_DEBUG_FS */ xenvif_disconnect_data(vif); - for (queue_index = 0; - queue_index < vif->num_queues; - ++queue_index) - xenvif_deinit_queue(&vif->queues[queue_index]); - spin_lock(&vif->lock); - queues = vif->queues; + /* At this point some of the handlers may still be active + * so we need to have additional synchronization here. + */ vif->num_queues = 0; - vif->queues = NULL; - spin_unlock(&vif->lock); + synchronize_net(); - vfree(queues); + for (queue_index = 0; queue_index < num_queues; ++queue_index) + xenvif_deinit_queue(&vif->queues[queue_index]); + + vfree(vif->queues); + vif->queues = NULL; xenvif_disconnect_ctrl(vif); }