From patchwork Mon Feb 22 14:10:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Vrabel X-Patchwork-Id: 586183 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C6918140BA3 for ; Tue, 23 Feb 2016 01:10:33 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755169AbcBVOKJ (ORCPT ); Mon, 22 Feb 2016 09:10:09 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:15808 "EHLO SMTP02.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754973AbcBVOKH (ORCPT ); Mon, 22 Feb 2016 09:10:07 -0500 X-IronPort-AV: E=Sophos;i="5.22,484,1449532800"; d="scan'208";a="339933821" Subject: Re: [Xen-devel] [PATCH] xen-netfront: set real_num_tx_queues to zreo avoid to trigger BUG_ON To: Gonglei , , , References: <1455931646-5672-1-git-send-email-arei.gonglei@huawei.com> CC: , "David S . Miller" From: David Vrabel X-Enigmail-Draft-Status: N1110 Message-ID: <56CB16BA.3020102@citrix.com> Date: Mon, 22 Feb 2016 14:10:02 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <1455931646-5672-1-git-send-email-arei.gonglei@huawei.com> X-DLP: MIA1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 20/02/16 01:27, Gonglei wrote: > It's possible for a race condition to exist between xennet_open() and > talk_to_netback(). After invoking netfront_probe() then other > threads or processes invoke xennet_open (such as NetworkManager) > immediately may trigger BUG_ON(). Besides, we also should reset > real_num_tx_queues in xennet_destroy_queues(). I think you want something like the following. We serialize the backend changing and the netdev open with the device mutex. (I've not even tried to compile this so it might not even build.) David diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index d6abf19..7e2791a 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -340,22 +340,28 @@ static int xennet_open(struct net_device *dev) unsigned int i = 0; struct netfront_queue *queue = NULL; + device_lock(&np->xbdev->dev); + + if (!netif_carrier_ok(dev)) + goto unlock; + for (i = 0; i < num_queues; ++i) { queue = &np->queues[i]; napi_enable(&queue->napi); spin_lock_bh(&queue->rx_lock); - if (netif_carrier_ok(dev)) { - xennet_alloc_rx_buffers(queue); - queue->rx.sring->rsp_event = queue->rx.rsp_cons + 1; - if (RING_HAS_UNCONSUMED_RESPONSES(&queue->rx)) - napi_schedule(&queue->napi); - } + xennet_alloc_rx_buffers(queue); + queue->rx.sring->rsp_event = queue->rx.rsp_cons + 1; + if (RING_HAS_UNCONSUMED_RESPONSES(&queue->rx)) + napi_schedule(&queue->napi); spin_unlock_bh(&queue->rx_lock); } netif_tx_start_all_queues(dev); + unlock: + device_unlock(&np->xbdev->dev); + return 0; } @@ -1983,8 +1989,8 @@ static int xennet_connect(struct net_device *dev) num_queues = dev->real_num_tx_queues; rtnl_lock(); + netdev_update_features(dev); - rtnl_unlock(); /* * All public and private state should now be sane. Get @@ -1992,7 +1998,6 @@ static int xennet_connect(struct net_device *dev) * domain a kick because we've probably just requeued some * packets. */ - netif_carrier_on(np->netdev); for (j = 0; j < num_queues; ++j) { queue = &np->queues[j]; @@ -2006,9 +2011,17 @@ static int xennet_connect(struct net_device *dev) spin_lock_bh(&queue->rx_lock); xennet_alloc_rx_buffers(queue); + if (netif_running(dev)) { + if (RING_HAS_UNCONSUMED_RESPONSES(&queue->rx)) + napi_schedule(&queue->napi); + } spin_unlock_bh(&queue->rx_lock); } + netif_carrier_on(np->netdev); + + rtnl_unlock(); + return 0; } diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index 33a31cf..57cd20d 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -204,8 +204,11 @@ void xenbus_otherend_changed(struct xenbus_watch *watch, return; } - if (drv->otherend_changed) + if (drv->otherend_changed) { + device_lock(&dev->dev); drv->otherend_changed(dev, state); + device_unlock(&dev->dev); + } } EXPORT_SYMBOL_GPL(xenbus_otherend_changed);