Skip to contentSkip to conversation

fix serial token buket

PATCH In review Stefano Stabellini 1 participant 2 messages Nov 14, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix serial token buket

Using timespec in the serial token bucket causes problems with integer overflows.
This patch fixes the issue using timeval instead of timespec variables, everywhere but from reading the current time with clock_gettime.

---

hw/serial.c +21 -18
diff -r 0ea6bd53cfb6 hw/serial.c
@@ -155,8 +155,8 @@
    doesn't kill dom0.  Simple token bucket.  If we get some actual
    data from the user, instantly refil the bucket. */
 
-/* How long it takes to generate a token, in nanoseconds. */
-#define TOKEN_PERIOD 1000000
+/* How long it takes to generate a token, in microseconds. */
+#define TOKEN_PERIOD 1000
 /* Maximum and initial size of token bucket */
 #define TOKENS_MAX 100000
 
@@ -279,16 +279,19 @@
 
 static void serial_get_token(void)
 {
-    static struct timespec last_refil_time;
+    static struct timeval last_refil_time;
     static int started;
 
     assert(tokens_avail >= 0);
     if (!tokens_avail) {
-	struct timespec delta, now;
+	struct timespec now;
+	struct timeval delta;
 	long generated;
 
 	if (!started) {
-	    clock_gettime(CLOCK_MONOTONIC, &last_refil_time);
+	    clock_gettime(CLOCK_MONOTONIC, &now);
+	    last_refil_time.tv_sec = now.tv_sec;
+	    last_refil_time.tv_usec = now.tv_nsec / 1000;
 	    tokens_avail = TOKENS_MAX;
 	    started = 1;
 	    return;
@@ -296,17 +299,17 @@
     retry:
 	clock_gettime(CLOCK_MONOTONIC, &now);
 	delta.tv_sec = now.tv_sec - last_refil_time.tv_sec;
-	delta.tv_nsec = now.tv_nsec - last_refil_time.tv_nsec;
-	if (delta.tv_nsec < 0) {
-	    delta.tv_nsec += 1000000000;
+	delta.tv_usec = (now.tv_nsec / 1000) - last_refil_time.tv_usec;
+	if (delta.tv_usec < 0) {
+	    delta.tv_usec += 1000000;
 	    delta.tv_sec--;
 	}
-	assert(delta.tv_nsec >= 0 && delta.tv_sec >= 0);
-	if (delta.tv_nsec < TOKEN_PERIOD) {
+	assert(delta.tv_usec >= 0 && delta.tv_sec >= 0);
+	if (delta.tv_sec == 0 && delta.tv_usec < TOKEN_PERIOD) {
 	    struct timespec ts;
 	    /* Wait until at least one token is available. */
-	    ts.tv_sec = TOKEN_PERIOD / 1000000000;
-	    ts.tv_nsec = TOKEN_PERIOD % 1000000000;
+	    ts.tv_sec = TOKEN_PERIOD / 1000000;
+	    ts.tv_nsec = (TOKEN_PERIOD % 1000000) * 1000;
 	    while (nanosleep(&ts, &ts) < 0 && errno == EINTR)
 		;
 	    goto retry;
@@ -316,15 +319,15 @@
             delta.tv_sec = 2;
             delta.tv_nsec = 0;
         }
-	generated = (delta.tv_sec * 1000000000) / TOKEN_PERIOD;
+	generated = (delta.tv_sec * 1000000) / TOKEN_PERIOD;
 	generated +=
-	    ((delta.tv_sec * 1000000000) % TOKEN_PERIOD + delta.tv_nsec) / TOKEN_PERIOD;
+	    ((delta.tv_sec * 1000000) % TOKEN_PERIOD + delta.tv_usec) / TOKEN_PERIOD;
 	assert(generated > 0);
 
-	last_refil_time.tv_nsec += (generated * TOKEN_PERIOD) % 1000000000;
-	last_refil_time.tv_sec  += last_refil_time.tv_nsec / 1000000000;
-	last_refil_time.tv_nsec %= 1000000000;
-        last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000000;
+	last_refil_time.tv_usec += (generated * TOKEN_PERIOD) % 1000000;
+	last_refil_time.tv_sec  += last_refil_time.tv_usec / 1000000;
+	last_refil_time.tv_usec %= 1000000;
+	last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000;
 	if (generated > TOKENS_MAX)
 	    generated = TOKENS_MAX;
 	tokens_avail = generated;
#1 lore raw
Quoted 5 lines · Stefano Stabellini

Using timespec in the serial token bucket causes problems with integer overflows.
This patch fixes the issue using timeval instead of timespec variables, everywhere but from reading the current time with clock_gettime.

Oops, I forgot one substitution.

---

hw/serial.c +24 -21
diff -r 0ea6bd53cfb6 hw/serial.c
@@ -155,8 +155,8 @@
    doesn't kill dom0.  Simple token bucket.  If we get some actual
    data from the user, instantly refil the bucket. */
 
-/* How long it takes to generate a token, in nanoseconds. */
-#define TOKEN_PERIOD 1000000
+/* How long it takes to generate a token, in microseconds. */
+#define TOKEN_PERIOD 1000
 /* Maximum and initial size of token bucket */
 #define TOKENS_MAX 100000
 
@@ -279,16 +279,19 @@
 
 static void serial_get_token(void)
 {
-    static struct timespec last_refil_time;
+    static struct timeval last_refil_time;
     static int started;
 
     assert(tokens_avail >= 0);
     if (!tokens_avail) {
-	struct timespec delta, now;
+	struct timespec now;
+	struct timeval delta;
 	long generated;
 
 	if (!started) {
-	    clock_gettime(CLOCK_MONOTONIC, &last_refil_time);
+	    clock_gettime(CLOCK_MONOTONIC, &now);
+	    last_refil_time.tv_sec = now.tv_sec;
+	    last_refil_time.tv_usec = now.tv_nsec / 1000;
 	    tokens_avail = TOKENS_MAX;
 	    started = 1;
 	    return;
@@ -296,35 +299,35 @@
     retry:
 	clock_gettime(CLOCK_MONOTONIC, &now);
 	delta.tv_sec = now.tv_sec - last_refil_time.tv_sec;
-	delta.tv_nsec = now.tv_nsec - last_refil_time.tv_nsec;
-	if (delta.tv_nsec < 0) {
-	    delta.tv_nsec += 1000000000;
+	delta.tv_usec = (now.tv_nsec / 1000) - last_refil_time.tv_usec;
+	if (delta.tv_usec < 0) {
+	    delta.tv_usec += 1000000;
 	    delta.tv_sec--;
 	}
-	assert(delta.tv_nsec >= 0 && delta.tv_sec >= 0);
-	if (delta.tv_nsec < TOKEN_PERIOD) {
+	assert(delta.tv_usec >= 0 && delta.tv_sec >= 0);
+	if (delta.tv_sec == 0 && delta.tv_usec < TOKEN_PERIOD) {
 	    struct timespec ts;
 	    /* Wait until at least one token is available. */
-	    ts.tv_sec = TOKEN_PERIOD / 1000000000;
-	    ts.tv_nsec = TOKEN_PERIOD % 1000000000;
+	    ts.tv_sec = TOKEN_PERIOD / 1000000;
+	    ts.tv_nsec = (TOKEN_PERIOD % 1000000) * 1000;
 	    while (nanosleep(&ts, &ts) < 0 && errno == EINTR)
 		;
 	    goto retry;
 	}
-        if (delta.tv_sec >= 2) {
+        if (delta.tv_sec >= 2000) {
             /* avoid arithmetic overflow if it has been ages */
-            delta.tv_sec = 2;
-            delta.tv_nsec = 0;
+            delta.tv_sec = 2000;
+            delta.tv_usec = 0;
         }
-	generated = (delta.tv_sec * 1000000000) / TOKEN_PERIOD;
+	generated = (delta.tv_sec * 1000000) / TOKEN_PERIOD;
 	generated +=
-	    ((delta.tv_sec * 1000000000) % TOKEN_PERIOD + delta.tv_nsec) / TOKEN_PERIOD;
+	    ((delta.tv_sec * 1000000) % TOKEN_PERIOD + delta.tv_usec) / TOKEN_PERIOD;
 	assert(generated > 0);
 
-	last_refil_time.tv_nsec += (generated * TOKEN_PERIOD) % 1000000000;
-	last_refil_time.tv_sec  += last_refil_time.tv_nsec / 1000000000;
-	last_refil_time.tv_nsec %= 1000000000;
-        last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000000;
+	last_refil_time.tv_usec += (generated * TOKEN_PERIOD) % 1000000;
+	last_refil_time.tv_sec  += last_refil_time.tv_usec / 1000000;
+	last_refil_time.tv_usec %= 1000000;
+	last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000;
 	if (generated > TOKENS_MAX)
 	    generated = TOKENS_MAX;
 	tokens_avail = generated;