diff options
| author | Michael S. Tsirkin <mst@redhat.com> | 2012-07-02 10:33:08 +0300 |
|---|---|---|
| committer | Rusty Russell <rusty@rustcorp.com.au> | 2012-07-09 09:07:22 +0930 |
| commit | 9c378abc5c0c6fc8e3acf5968924d274503819b3 (patch) | |
| tree | a818c1e2cda775a8f52e11d35ae8f9b02cfd2522 /drivers/virtio | |
| parent | 02edf6abe01610a5fb379df442de3c837ad99467 (diff) | |
virtio-balloon: fix add/get API use
Since ee7cd8981e15bcb365fc762afe3fc47b8242f630 'virtio: expose added
descriptors immediately.', in virtio balloon virtqueue_get_buf might
now run concurrently with virtqueue_kick. I audited both and this
seems safe in practice but this is not guaranteed by the API.
Additionally, a spurious interrupt might in theory make
virtqueue_get_buf run in parallel with virtqueue_add_buf, which is
racy.
While we might try to protect against spurious callbacks it's
easier to fix the driver: balloon seems to be the only one
(mis)using the API like this, so let's just fix balloon.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (removed unused var)
Diffstat (limited to 'drivers/virtio')
| -rw-r--r-- | drivers/virtio/virtio_balloon.c | 24 |
1 files changed, 10 insertions, 14 deletions
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index bfbc15ca38d..0908e604433 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -47,7 +47,7 @@ struct virtio_balloon struct task_struct *thread; /* Waiting for host to ack the pages we released. */ - struct completion acked; + wait_queue_head_t acked; /* Number of balloon pages we've told the Host we're not using. */ unsigned int num_pages; @@ -89,29 +89,25 @@ static struct page *balloon_pfn_to_page(u32 pfn) static void balloon_ack(struct virtqueue *vq) { - struct virtio_balloon *vb; - unsigned int len; + struct virtio_balloon *vb = vq->vdev->priv; - vb = virtqueue_get_buf(vq, &len); - if (vb) - complete(&vb->acked); + wake_up(&vb->acked); } static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) { struct scatterlist sg; + unsigned int len; sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns); - init_completion(&vb->acked); - /* We should always be able to add one buffer to an empty queue. */ if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) BUG(); virtqueue_kick(vq); /* When host has read buffer, this completes via balloon_ack */ - wait_for_completion(&vb->acked); + wait_event(vb->acked, virtqueue_get_buf(vq, &len)); } static void set_page_pfns(u32 pfns[], struct page *page) @@ -231,12 +227,8 @@ static void update_balloon_stats(struct virtio_balloon *vb) */ static void stats_request(struct virtqueue *vq) { - struct virtio_balloon *vb; - unsigned int len; + struct virtio_balloon *vb = vq->vdev->priv; - vb = virtqueue_get_buf(vq, &len); - if (!vb) - return; vb->need_stats_update = 1; wake_up(&vb->config_change); } @@ -245,11 +237,14 @@ static void stats_handle_request(struct virtio_balloon *vb) { struct virtqueue *vq; struct scatterlist sg; + unsigned int len; vb->need_stats_update = 0; update_balloon_stats(vb); vq = vb->stats_vq; + if (!virtqueue_get_buf(vq, &len)) + return; sg_init_one(&sg, vb->stats, sizeof(vb->stats)); if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) BUG(); @@ -358,6 +353,7 @@ static int virtballoon_probe(struct virtio_device *vdev) INIT_LIST_HEAD(&vb->pages); vb->num_pages = 0; init_waitqueue_head(&vb->config_change); + init_waitqueue_head(&vb->acked); vb->vdev = vdev; vb->need_stats_update = 0; |