[lkml]   [2012]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRFD: virtio balloon API use (was Re: [PATCH 5 of 5] virtio: expose added descriptors immediately)
On Thu, Nov 03, 2011 at 06:12:53PM +1030, Rusty Russell wrote:
> A virtio driver does virtqueue_add_buf() multiple times before finally
> calling virtqueue_kick(); previously we only exposed the added buffers
> in the virtqueue_kick() call. This means we don't need a memory
> barrier in virtqueue_add_buf(), but it reduces concurrency as the
> device (ie. host) can't see the buffers until the kick.
> Signed-off-by: Rusty Russell <>

Looking at recent mm compaction patches made me look at locking
in balloon closely. And I noticed the referenced patch (commit
ee7cd8981e15bcb365fc762afe3fc47b8242f630 upstream) interacts strangely
with virtio balloon; balloon currently does:

static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
struct scatterlist sg;

sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);


/* 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)

/* When host has read buffer, this completes via balloon_ack */

While vq callback does:

static void balloon_ack(struct virtqueue *vq)
struct virtio_balloon *vb;
unsigned int len;

vb = virtqueue_get_buf(vq, &len);
if (vb)

So virtqueue_get_buf might now run concurrently with virtqueue_kick.
I audited both and this seems safe in practice but I think
we need to either declare this legal at the API level
or add locking in driver.

Further, is there a guarantee that we never get
spurious callbacks? We currently check ring not empty
but esp for non shared MSI this might not be needed.
If a spurious callback triggers, virtqueue_get_buf can run
concurrently with virtqueue_add_buf which is known to be racy.
Again I think this is currently safe as no spurious callbacks in
practice but should we guarantee no spurious callbacks at the API level
or add locking in driver?


 \ /
  Last update: 2012-07-01 12:01    [W:0.120 / U:15.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site