From: "Michael S. Tsirkin" <mst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
netdev@vger.kernel.org,
virtualization@lists.linux-foundation.org, qinchuanyu@huawei.com,
davem@davemloft.net
Subject: [PATCH net 2/3] vhost: fix ref cnt checking deadlock
Date: Wed, 12 Feb 2014 18:38:00 +0200 [thread overview]
Message-ID: <1392222846-26699-3-git-send-email-mst@redhat.com> (raw)
In-Reply-To: <1392222846-26699-1-git-send-email-mst@redhat.com>
vhost checked the counter within the refcnt before decrementing. It
really wanted to know that there aren't too many references, as a way to
batch freeing resources a bit more efficiently.
This works well but it we now access the
ref counter twice so there's a race:
all users might see a high count and decide
to defer freeing resources.
In the end no one initiates freeing resources
until the last reference is gone (which is on VM shotdown
so might happen after a looooong time).
Let's do what we should have done straight away:
add a kref API to return the kref value atomically,
and use that to avoid the deadlock.
Reported-by: Qin Chuanyu <qinchuanyu@huawei.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 831eb4f..7eaf2de 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -140,9 +140,9 @@ vhost_net_ubuf_alloc(struct vhost_virtqueue *vq, bool zcopy)
return ubufs;
}
-static void vhost_net_ubuf_put(struct vhost_net_ubuf_ref *ubufs)
+static int vhost_net_ubuf_put(struct vhost_net_ubuf_ref *ubufs)
{
- kref_put(&ubufs->kref, vhost_net_zerocopy_done_signal);
+ return kref_sub_return(&ubufs->kref, 1, vhost_net_zerocopy_done_signal);
}
static void vhost_net_ubuf_put_and_wait(struct vhost_net_ubuf_ref *ubufs)
@@ -306,22 +306,21 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
{
struct vhost_net_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
- int cnt = atomic_read(&ubufs->kref.refcount);
+ int cnt;
/* set len to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = success ?
VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
- vhost_net_ubuf_put(ubufs);
+ cnt = vhost_net_ubuf_put(ubufs);
/*
* Trigger polling thread if guest stopped submitting new buffers:
- * in this case, the refcount after decrement will eventually reach 1
- * so here it is 2.
+ * in this case, the refcount after decrement will eventually reach 1.
* We also trigger polling periodically after each 16 packets
* (the value 16 here is more or less arbitrary, it's tuned to trigger
* less than 10% of times).
*/
- if (cnt <= 2 || !(cnt % 16))
+ if (cnt <= 1 || !(cnt % 16))
vhost_poll_queue(&vq->poll);
}
--
MST
next prev parent reply other threads:[~2014-02-12 16:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-12 16:37 [PATCH net 0/3] vhost fixes for 3.14, -stable Michael S. Tsirkin
2014-02-12 16:36 ` [PATCH net 3/3] vhost: fix a theoretical race in device cleanup Michael S. Tsirkin
2014-02-12 16:38 ` Michael S. Tsirkin [this message]
2014-02-12 16:38 ` [PATCH net 1/3] kref: add kref_sub_return Michael S. Tsirkin
2014-02-12 16:56 ` Greg Kroah-Hartman
2014-02-12 17:35 ` Michael S. Tsirkin
[not found] ` <20140212173524.GA26860@redhat.com>
2014-02-12 18:37 ` Greg Kroah-Hartman
2014-02-12 18:39 ` Anatol Pomozov
2014-02-13 0:06 ` David Miller
2014-02-13 1:25 ` Jörn Engel
2014-02-13 1:39 ` Greg KH
2014-02-13 4:05 ` David Miller
2014-02-13 4:09 ` David Miller
2014-02-14 0:03 ` Greg KH
2014-02-14 5:10 ` 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=1392222846-26699-3-git-send-email-mst@redhat.com \
--to=mst@redhat.com \
--cc=davem@davemloft.net \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=qinchuanyu@huawei.com \
--cc=virtio-dev@lists.oasis-open.org \
--cc=virtualization@lists.linux-foundation.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).