monitor: Simplify event throttling

The event throttling state machine is hard to understand.  I'm not
sure it's entirely correct.  Rewrite it in a more straightforward
manner:

State 1: No event sent recently (less than evconf->rate ns ago)

    Invariant: evstate->timer is not pending, evstate->qdict is null

    On event: send event, arm timer, goto state 2

State 2: Event sent recently, no additional event being delayed

    Invariant: evstate->timer is pending, evstate->qdict is null

    On event: store it in evstate->qdict, goto state 3

    On timer: goto state 1

State 3: Event sent recently, additional event being delayed

    Invariant: evstate->timer is pending, evstate->qdict is non-null

    On event: store it in evstate->qdict, goto state 3

    On timer: send evstate->qdict, clear evstate->qdict,
              arm timer, goto state 2

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444921716-9511-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

2 files changed, 32 insertions, 35 deletions

diff --git a/monitor.c b/monitor.c
index f90a42f..ec4096b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -183,7 +183,6 @@ typedef struct {
 typedef struct MonitorQAPIEventState {
     QAPIEvent event;    /* Event being tracked */
     int64_t rate;       /* Minimum time (in ns) between two events */
-    int64_t last;       /* QEMU_CLOCK_REALTIME value at last emission */
     QEMUTimer *timer;   /* Timer for handling delayed events */
     QDict *qdict;       /* Delayed event (if any) */
 } MonitorQAPIEventState;
@@ -464,65 +463,64 @@ static void
 monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
 {
     MonitorQAPIEventState *evstate;
-    assert(event < QAPI_EVENT_MAX);
-    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 
-    evstate = &(monitor_qapi_event_state[event]);
-    trace_monitor_protocol_event_queue(event,
-                                       qdict,
-                                       evstate->rate,
-                                       evstate->last,
-                                       now);
+    assert(event < QAPI_EVENT_MAX);
+    evstate = &monitor_qapi_event_state[event];
+    trace_monitor_protocol_event_queue(event, qdict, evstate->rate);
 
-    /* Rate limit of 0 indicates no throttling */
     qemu_mutex_lock(&monitor_lock);
+
     if (!evstate->rate) {
+        /* Unthrottled event */
         monitor_qapi_event_emit(event, qdict);
-        evstate->last = now;
     } else {
-        int64_t delta = now - evstate->last;
-        if (evstate->qdict ||
-            delta < evstate->rate) {
-            /* If there's an existing event pending, replace
-             * it with the new event, otherwise schedule a
-             * timer for delayed emission
+        if (timer_pending(evstate->timer)) {
+            /*
+             * Timer is pending for (at least) evstate->rate ns after
+             * last send.  Store event for sending when timer fires,
+             * replacing a prior stored event if any.
              */
-            if (evstate->qdict) {
-                QDECREF(evstate->qdict);
-            } else {
-                int64_t then = evstate->last + evstate->rate;
-                timer_mod_ns(evstate->timer, then);
-            }
+            QDECREF(evstate->qdict);
             evstate->qdict = qdict;
             QINCREF(evstate->qdict);
         } else {
+            /*
+             * Last send was (at least) evstate->rate ns ago.
+             * Send immediately, and arm the timer to call
+             * monitor_qapi_event_handler() in evstate->rate ns.  Any
+             * events arriving before then will be delayed until then.
+             */
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
             monitor_qapi_event_emit(event, qdict);
-            evstate->last = now;
+            timer_mod_ns(evstate->timer, now + evstate->rate);
         }
     }
+
     qemu_mutex_unlock(&monitor_lock);
 }
 
 /*
- * The callback invoked by QemuTimer when a delayed
- * event is ready to be emitted
+ * This function runs evstate->rate ns after sending a throttled
+ * event.
+ * If another event has since been stored, send it.
  */
 static void monitor_qapi_event_handler(void *opaque)
 {
     MonitorQAPIEventState *evstate = opaque;
-    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 
-    trace_monitor_protocol_event_handler(evstate->event,
-                                         evstate->qdict,
-                                         evstate->last,
-                                         now);
+    trace_monitor_protocol_event_handler(evstate->event, evstate->qdict);
     qemu_mutex_lock(&monitor_lock);
+
     if (evstate->qdict) {
+        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
         monitor_qapi_event_emit(evstate->event, evstate->qdict);
         QDECREF(evstate->qdict);
         evstate->qdict = NULL;
+        timer_mod_ns(evstate->timer, now + evstate->rate);
     }
-    evstate->last = now;
+
     qemu_mutex_unlock(&monitor_lock);
 }
 
@@ -546,7 +544,6 @@ monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
     evstate->event = event;
     assert(rate * SCALE_MS <= INT64_MAX);
     evstate->rate = rate * SCALE_MS;
-    evstate->last = 0;
     evstate->qdict = NULL;
     evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
                                SCALE_MS,
diff --git a/trace-events b/trace-events
index bdfe79f..72136b9 100644
--- a/trace-events
+++ b/trace-events
@@ -1031,9 +1031,9 @@ esp_pci_sbac_write(uint32_t reg, uint32_t val) "sbac: 0x%8.8x -> 0x%8.8x"
 # monitor.c
 handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\""
 monitor_protocol_emitter(void *mon) "mon %p"
-monitor_protocol_event_handler(uint32_t event, void *data, uint64_t last, uint64_t now) "event=%d data=%p last=%" PRId64 " now=%" PRId64
+monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p"
 monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
-monitor_protocol_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
+monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event=%d data=%p rate=%" PRId64
 monitor_protocol_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
 
 # hw/net/opencores_eth.c