Re: [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation

["Alex Bennée" <alex.bennee@linaro.org>]

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch provides the vhost-user backend implementation to work
> in tandem with the vhost-user-rng implementation of the QEMU VMM.
>
> It uses the vhost-user API so that other VMM can re-use the interface
> without having to write the driver again.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Try the following patch which creates a nested main loop and runs it
until the g_timeout_add fires again.

--8<---------------cut here---------------start------------->8---
tools/virtio/vhost-user-rng: avoid mutex by using nested main loop

As we are blocking anyway all we really need to do is run a main loop
until the timer fires and the data is consumed.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

1 file changed, 30 insertions(+), 76 deletions(-)
tools/vhost-user-rng/main.c | 106 +++++++++++++-------------------------------

modified   tools/vhost-user-rng/main.c
@@ -42,13 +42,10 @@
 
 typedef struct {
     VugDev dev;
-    struct itimerspec ts;
-    timer_t rate_limit_timer;
-    pthread_mutex_t rng_mutex;
-    pthread_cond_t rng_cond;
     int64_t quota_remaining;
-    bool activate_timer;
+    guint timer;
     GMainLoop *loop;
+    GMainLoop *blocked;
 } VuRNG;
 
 static gboolean print_cap, verbose;
@@ -59,66 +56,26 @@ static gint source_fd, socket_fd = -1;
 static uint32_t period_ms = 1 << 16;
 static uint64_t max_bytes = INT64_MAX;
 
-static void check_rate_limit(union sigval sv)
+static gboolean check_rate_limit(gpointer user_data)
 {
-    VuRNG *rng = sv.sival_ptr;
-    bool wakeup = false;
+    VuRNG *rng = (VuRNG *) user_data;
 
-    pthread_mutex_lock(&rng->rng_mutex);
-    /*
-     * The timer has expired and the guest has used all available
-     * entropy, which means function vu_rng_handle_request() is waiting
-     * on us.  As such wake it up once we're done here.
-     */
-    if (rng->quota_remaining == 0) {
-        wakeup = true;
+    if (rng->blocked) {
+        g_info("%s: called while blocked", __func__);
+        g_main_loop_quit(rng->blocked);
     }
-
     /*
      * Reset the entropy available to the guest and tell function
      * vu_rng_handle_requests() to start the timer before using it.
      */
     rng->quota_remaining = max_bytes;
-    rng->activate_timer = true;
-    pthread_mutex_unlock(&rng->rng_mutex);
-
-    if (wakeup) {
-        pthread_cond_signal(&rng->rng_cond);
-    }
-}
-
-static void setup_timer(VuRNG *rng)
-{
-    struct sigevent sev;
-    int ret;
-
-    memset(&rng->ts, 0, sizeof(struct itimerspec));
-    rng->ts.it_value.tv_sec = period_ms / 1000;
-    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
-
-    /*
-     * Call function check_rate_limit() as if it was the start of
-     * a new thread when the timer expires.
-     */
-    sev.sigev_notify = SIGEV_THREAD;
-    sev.sigev_notify_function = check_rate_limit;
-    sev.sigev_value.sival_ptr = rng;
-    /* Needs to be NULL if defaults attributes are to be used. */
-    sev.sigev_notify_attributes = NULL;
-    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
-    if (ret < 0) {
-        fprintf(stderr, "timer_create() failed\n");
-    }
-
+    return true;
 }
 
-
 /* Virtio helpers */
 static uint64_t rng_get_features(VuDev *dev)
 {
-    if (verbose) {
-        g_info("%s: replying", __func__);
-    }
+    g_info("%s: replying", __func__);
     return 0;
 }
 
@@ -137,7 +94,7 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
     VuVirtq *vq = vu_get_queue(dev, qidx);
     VuVirtqElement *elem;
     size_t to_read;
-    int len, ret;
+    int len;
 
     for (;;) {
         /* Get element in the vhost virtqueue */
@@ -149,24 +106,21 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
         /* Get the amount of entropy to read from the vhost server */
         to_read = elem->in_sg[0].iov_len;
 
-        pthread_mutex_lock(&rng->rng_mutex);
-
         /*
          * We have consumed all entropy available for this time slice.
          * Wait for the timer (check_rate_limit()) to tell us about the
          * start of a new time slice.
          */
         if (rng->quota_remaining == 0) {
-            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
-        }
-
-        /* Start the timer if the last time slice has expired */
-        if (rng->activate_timer == true) {
-            rng->activate_timer = false;
-            ret = timer_settime(rng->rate_limit_timer, 0, &rng->ts, NULL);
-            if (ret < 0) {
-                fprintf(stderr, "timer_settime() failed\n");
-            }
+            g_assert(!rng->blocked);
+            rng->blocked = g_main_loop_new(g_main_loop_get_context(rng->loop), false);
+            g_info("attempting to consume %ld bytes but no quota left (%s)",
+                   to_read,
+                   g_main_loop_is_running(rng->loop) ? "running" : "not running");
+            g_main_loop_run(rng->blocked);
+            g_info("return from blocked loop: %ld", rng->quota_remaining);
+            g_main_loop_unref(rng->blocked);
+            rng->blocked = false;
         }
 
         /* Make sure we don't read more than it's available */
@@ -183,8 +137,6 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
 
         rng->quota_remaining -= len;
 
-        pthread_mutex_unlock(&rng->rng_mutex);
-
         vu_queue_push(dev, vq, elem, len);
         free(elem);
     }
@@ -373,6 +325,7 @@ int main(int argc, char *argv[])
      * can add it's GSource watches.
      */
     rng.loop = g_main_loop_new(NULL, FALSE);
+    rng.blocked = NULL;
 
     if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
                   panic, &vuiface)) {
@@ -380,24 +333,25 @@ int main(int argc, char *argv[])
         exit(EXIT_FAILURE);
     }
 
-    rng.quota_remaining = max_bytes;
-    rng.activate_timer = true;
-    pthread_mutex_init(&rng.rng_mutex, NULL);
-    pthread_cond_init(&rng.rng_cond, NULL);
-    setup_timer(&rng);
-
     if (verbose) {
-        g_info("period_ms: %d tv_sec: %ld tv_nsec: %lu\n",
-               period_ms, rng.ts.it_value.tv_sec, rng.ts.it_value.tv_nsec);
+        g_log_set_handler(NULL, G_LOG_LEVEL_MASK, g_log_default_handler, NULL);
+        g_setenv("G_MESSAGES_DEBUG", "all", true);
+    } else {
+        g_log_set_handler(NULL,
+                          G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
+                          g_log_default_handler, NULL);
     }
 
+    rng.quota_remaining = max_bytes;
+    rng.timer = g_timeout_add(period_ms, check_rate_limit, &rng);
+    g_info("period_ms: %"PRId32", timer %d\n", period_ms, rng.timer);
+
     g_message("entering main loop, awaiting messages");
     g_main_loop_run(rng.loop);
     g_message("finished main loop, cleaning up");
 
     g_main_loop_unref(rng.loop);
     vug_deinit(&rng.dev);
-    timer_delete(rng.rate_limit_timer);
     close(source_fd);
     unlink(socket_path);
 }
--8<---------------cut here---------------end--------------->8---

-- 
Alex Bennée