qio: Only fire writable taps on edge transitions
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 6 Oct 2016 15:56:58 +0000 (11:56 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 6 Oct 2016 19:41:48 +0000 (15:41 -0400)
We were firing writable taps (via the qwake_cb()) any time someone read
from a queue.  The effect of this was that applications that tapped their
Qdata FD would see a lot of writable taps firing, even though there wasn't
an edge transition.

For instance, say a conversation's write queue (outbound, TX) is no where
near full.  The app puts a packet in the queue.  When the network stack
drains the block from the queue with __qbread(), that will trigger a
writable tap.  So the app gets an FD tap / epoll every time it writes a
packet.  Incidentally, that behavior helped me track down a bug, but it
isn't what we're looking for.

Like the read side, we only fire on edge transitions, as done in commit
dbaaf4a3029e ("qio: Fire read taps on actual edges").  Back then, I had us
firing writable taps all the time, which was a bit much.

Note that we still fire the readable/writable taps regardless of
Qstarve/Qflow.  Those queue state flags only get set when someone tries to
read/write a queue and fails.  The taps we fire occur independently, which
is why their logic (e.g. was_empty / was_unwritable) are separate from the
rendez control variables (e.g. dowakeup).  This is probably right, since
it's possible for an application to know a queue would block without trying
(perhaps through stat()).

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/ns/qio.c

index fed4c2f..7f9b3ef 100644 (file)
@@ -107,7 +107,6 @@ static ssize_t __qbwrite(struct queue *q, struct block *b, int flags);
 static struct block *__qbread(struct queue *q, size_t len, int qio_flags,
                               int mem_flags);
 static bool qwait_and_ilock(struct queue *q, int qio_flags);
-static void qwakeup_iunlock(struct queue *q, int qio_flags);
 
 /* Helper: fires a wake callback, sending 'filter' */
 static void qwake_cb(struct queue *q, int filter)
@@ -734,6 +733,8 @@ static int __try_qbread(struct queue *q, size_t len, int qio_flags,
 {
        struct block *ret, *ret_last, *first;
        size_t blen;
+       bool was_unwritable = FALSE;
+       int dowakeup = 0;
 
        if (qio_flags & QIO_CAN_ERR_SLEEP) {
                if (!qwait_and_ilock(q, qio_flags)) {
@@ -750,6 +751,7 @@ static int __try_qbread(struct queue *q, size_t len, int qio_flags,
                        return QBR_FAIL;
                }
        }
+       was_unwritable = !qwritable(q);
        blen = BLEN(first);
        if ((q->state & Qcoalesce) && (blen == 0)) {
                freeb(pop_first_block(q));
@@ -806,7 +808,29 @@ static int __try_qbread(struct queue *q, size_t len, int qio_flags,
                len -= blen;
        }
 out_ok:
-       qwakeup_iunlock(q, qio_flags);
+       /*
+        *  if writer flow controlled, restart
+        *
+        *  This used to be
+        *  q->len < q->limit/2
+        *  but it slows down tcp too much for certain write sizes.
+        *  I really don't understand it completely.  It may be
+        *  due to the queue draining so fast that the transmission
+        *  stalls waiting for the app to produce more data.  - presotto
+        */
+       if ((q->state & Qflow) && q->len < q->limit) {
+               q->state &= ~Qflow;
+               dowakeup = 1;
+       }
+       spin_unlock_irqsave(&q->lock);
+       /* wakeup flow controlled writers */
+       if (dowakeup) {
+               if (q->kick && !(qio_flags & QIO_DONT_KICK))
+                       q->kick(q->arg);
+               rendez_wakeup(&q->wr);
+       }
+       if (was_unwritable)
+               qwake_cb(q, FDTAP_FILT_WRITABLE);
        *real_ret = ret;
        return QBR_OK;
 }
@@ -1377,40 +1401,6 @@ void qputback(struct queue *q, struct block *b)
 }
 
 /*
- *  flow control, get producer going again
- *  called with q ilocked
- */
-static void qwakeup_iunlock(struct queue *q, int qio_flags)
-{
-       int dowakeup = 0;
-
-       /*
-        *  if writer flow controlled, restart
-        *
-        *  This used to be
-        *  q->len < q->limit/2
-        *  but it slows down tcp too much for certain write sizes.
-        *  I really don't understand it completely.  It may be
-        *  due to the queue draining so fast that the transmission
-        *  stalls waiting for the app to produce more data.  - presotto
-        */
-       if ((q->state & Qflow) && q->len < q->limit) {
-               q->state &= ~Qflow;
-               dowakeup = 1;
-       }
-
-       spin_unlock_irqsave(&q->lock);
-
-       /* wakeup flow controlled writers */
-       if (dowakeup) {
-               if (q->kick && !(qio_flags & QIO_DONT_KICK))
-                       q->kick(q->arg);
-               rendez_wakeup(&q->wr);
-       }
-       qwake_cb(q, FDTAP_FILT_WRITABLE);
-}
-
-/*
  *  get next block from a queue (up to a limit)
  *
  */