From: David Vrabel <david.vrabel@citrix.com>
To: <netdev@vger.kernel.org>
Cc: David Vrabel <david.vrabel@citrix.com>,
<xen-devel@lists.xenproject.org>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: [PATCH 1/2] xen-netfront: don't nest queue locks in xennet_connect()
Date: Wed, 2 Jul 2014 16:09:14 +0100 [thread overview]
Message-ID: <1404313755-27349-2-git-send-email-david.vrabel@citrix.com> (raw)
In-Reply-To: <1404313755-27349-1-git-send-email-david.vrabel@citrix.com>
The nesting of the per-queue rx_lock and tx_lock in xennet_connect()
is confusing to both humans and lockdep. The locking is safe because
this is the only place where the locks are nested in this way but
lockdep still warns.
Instead of adding the missing lockdep annotations, refactor the
locking to avoid the confusing nesting. This is still safe, because
the xenbus connection state changes are all serialized by the xenwatch
thread.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
---
drivers/net/xen-netfront.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 2ccb4a0..6a37d62 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2046,13 +2046,15 @@ static int xennet_connect(struct net_device *dev)
/* By now, the queue structures have been set up */
for (j = 0; j < num_queues; ++j) {
queue = &np->queues[j];
- spin_lock_bh(&queue->rx_lock);
- spin_lock_irq(&queue->tx_lock);
/* Step 1: Discard all pending TX packet fragments. */
+ spin_lock_irq(&queue->tx_lock);
xennet_release_tx_bufs(queue);
+ spin_unlock_irq(&queue->tx_lock);
/* Step 2: Rebuild the RX buffer freelist and the RX ring itself. */
+ spin_lock_bh(&queue->rx_lock);
+
for (requeue_idx = 0, i = 0; i < NET_RX_RING_SIZE; i++) {
skb_frag_t *frag;
const struct page *page;
@@ -2076,6 +2078,8 @@ static int xennet_connect(struct net_device *dev)
}
queue->rx.req_prod_pvt = requeue_idx;
+
+ spin_unlock_bh(&queue->rx_lock);
}
/*
@@ -2087,13 +2091,17 @@ static int xennet_connect(struct net_device *dev)
netif_carrier_on(np->netdev);
for (j = 0; j < num_queues; ++j) {
queue = &np->queues[j];
+
notify_remote_via_irq(queue->tx_irq);
if (queue->tx_irq != queue->rx_irq)
notify_remote_via_irq(queue->rx_irq);
- xennet_tx_buf_gc(queue);
- xennet_alloc_rx_buffers(queue);
+ spin_lock_irq(&queue->tx_lock);
+ xennet_tx_buf_gc(queue);
spin_unlock_irq(&queue->tx_lock);
+
+ spin_lock_bh(&queue->rx_lock);
+ xennet_alloc_rx_buffers(queue);
spin_unlock_bh(&queue->rx_lock);
}
--
1.7.10.4
next prev parent reply other threads:[~2014-07-02 15:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-02 15:09 [PATCHv1 net 0/2] xen-netfront: multi-queue related locking fixes David Vrabel
2014-07-02 15:09 ` David Vrabel [this message]
2014-07-02 17:36 ` [PATCH 1/2] xen-netfront: don't nest queue locks in xennet_connect() Boris Ostrovsky
2014-07-03 16:19 ` David Vrabel
2014-07-02 15:09 ` [PATCH 2/2] xen-netfront: call netif_carrier_off() only once when disconnecting David Vrabel
2014-07-08 18:21 ` [PATCHv1 net 0/2] xen-netfront: multi-queue related locking fixes David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1404313755-27349-2-git-send-email-david.vrabel@citrix.com \
--to=david.vrabel@citrix.com \
--cc=boris.ostrovsky@oracle.com \
--cc=konrad.wilk@oracle.com \
--cc=netdev@vger.kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).