dsched_fq - Refactor and clean; handle flushes
authorAlex Hornung <ahornung@gmail.com>
Thu, 1 Apr 2010 01:09:59 +0000 (01:09 +0000)
committerAlex Hornung <ahornung@gmail.com>
Thu, 15 Apr 2010 20:24:49 +0000 (20:24 +0000)
* Factor out an fq_drain which will either cancel or dispatch all the
  bios we have currently in all our fqp queues.

* Clean out old comments and code.

* Deal with flushes by not queuing them but rather letting dsched handle
  them. By returning EINVAL, dsched_queue will dispatch the flush
  itself.

sys/dsched/fq/dsched_fq.h
sys/dsched/fq/dsched_fq_core.c
sys/dsched/fq/dsched_fq_diskops.c
sys/dsched/fq/dsched_fq_procops.c

index 23117c6..ba10f74 100644 (file)
 #define        FQ_REBALANCE_TIMEOUT    1       /* in seconds */
 #define FQ_TOTAL_DISK_TIME     1000000*FQ_REBALANCE_TIMEOUT    /* in useconds */
 
-/*
-#define        FQ_INNERQ_INIT(x)       spin_init(&(x)->lock)
-#define        FQ_INNERQ_UNINIT(x)     spin_uninit(&(x)->lock)
-#define FQ_INNERQ_LOCK(x)      spin_lock_wr(&(x)->lock)
-#define        FQ_INNERQ_UNLOCK(x)     spin_unlock_wr(&(x)->lock)
-#define FQ_INNERQ_SLEEP(x,y,z) ssleep((x), &(x)->lock, 0, y, z)
-*/
 
+#define        FQ_DRAIN_CANCEL 0x1
+#define        FQ_DRAIN_FLUSH  0x2
 
 struct disk;
 struct proc;
@@ -175,6 +170,7 @@ void        fq_dereference_priv(struct dsched_fq_priv *fqp);
 void   fq_dereference_mpriv(struct dsched_fq_mpriv *fqmp);
 void   fq_dispatch(struct dsched_fq_dpriv *dpriv, struct bio *bio,
                        struct dsched_fq_priv *fqp);
+void   fq_drain(struct dsched_fq_dpriv *dpriv, int mode);
 #endif /* _KERNEL || _KERNEL_STRUCTURES */
 
 
index b08dc09..d87d634 100644 (file)
@@ -358,21 +358,7 @@ fq_dispatcher(struct dsched_fq_dpriv *dpriv)
 
                        if (__predict_false(dpriv->die == 1)) {
                                /* If we are supposed to die, drain all queues */
-                               TAILQ_FOREACH_MUTABLE(fqp, &dpriv->fq_priv_list,
-                                   dlink, fqp2) {
-                                       if (fqp->qlength == 0)
-                                               continue;
-
-                                       FQ_FQP_LOCK(fqp);
-                                       TAILQ_FOREACH_MUTABLE(bio, &fqp->queue,
-                                           link, bio2) {
-                                               TAILQ_REMOVE(&fqp->queue, bio,
-                                                   link);
-                                               --fqp->qlength;
-                                               fq_dispatch(dpriv, bio, fqp);
-                                       }
-                                       FQ_FQP_UNLOCK(fqp);
-                               }
+                               fq_drain(dpriv, FQ_DRAIN_FLUSH);
 
                                /* Now we can safely unlock and exit */
                                FQ_DPRIV_UNLOCK(dpriv);
@@ -458,26 +444,7 @@ fq_balance_thread(struct dsched_fq_dpriv *dpriv)
 
        TAILQ_FOREACH_MUTABLE(fqp, &dpriv->fq_priv_list, dlink, fqp2) {
                if (fqp->transactions > 0 /* 30 */) {
-
                        total_budget += (fqp->avg_latency * fqp->transactions);
-                       /*
-                        * XXX: while the code below really sucked, the problem needs to
-                        *      be addressed eventually. Some processes take up their "fair"
-                        *      slice, but don't really need even a 10th of it.
-                        *      This kills performance for those that do need the
-                        *      performance.
-                        */
-#if 0
-                       /*
-                        * This is *very* hackish. It basically tries to avoid that
-                        * processes that do only very few tps take away more bandwidth
-                        * than they should.
-                        */
-                       if ((limited_procs >= 1) && (fqp->transactions < 25) &&
-                           (budgetpb[(fqp->p) ? fqp->p->p_ionice : 0] >= 1))
-                               continue;
-#endif
-
                        ++budgetpb[(fqp->p) ? fqp->p->p_ionice : 0];
 
                        dsched_debug(LOG_INFO,
@@ -499,13 +466,16 @@ fq_balance_thread(struct dsched_fq_dpriv *dpriv)
                goto done;
 
        sum = 0;
+
        for (i = 0; i < FQ_PRIO_MAX+1; i++) {
                if (budgetpb[i] == 0)
                        continue;
                sum += (FQ_PRIO_BIAS+i)*budgetpb[i];
        }
+
        if (sum == 0)
                sum = 1;
+
        dsched_debug(LOG_INFO, "sum = %d\n", sum);
 
        for (i = 0; i < FQ_PRIO_MAX+1; i++) {
index ef2badc..886a999 100644 (file)
@@ -168,20 +168,51 @@ fq_teardown(struct disk *dp)
 }
 
 
-static void
-fq_flush(struct disk *dp, struct bio *biin)
+/* Must be called with locked dpriv */
+void
+fq_drain(struct dsched_fq_dpriv *dpriv, int mode)
 {
-       /* Our flushes are handled by queue, this should never be called */
+       struct dsched_fq_priv *fqp, *fqp2;
+       struct bio *bio, *bio2;
+
+       TAILQ_FOREACH_MUTABLE(fqp, &dpriv->fq_priv_list, dlink, fqp2) {
+               if (fqp->qlength == 0)
+                       continue;
+
+               FQ_FQP_LOCK(fqp);
+               TAILQ_FOREACH_MUTABLE(bio, &fqp->queue, link, bio2) {
+                       TAILQ_REMOVE(&fqp->queue, bio, link);
+                       --fqp->qlength;
+                       if (__predict_false(mode == FQ_DRAIN_CANCEL)) {
+                               /* FQ_DRAIN_CANCEL */
+                               dsched_cancel_bio(bio);
+                               atomic_add_int(&fq_stats.cancelled, 1);
+
+                               /* Release ref acquired on fq_queue */
+                               /* XXX: possible failure point */
+                               fq_dereference_priv(fqp);
+                       } else {
+                               /* FQ_DRAIN_FLUSH */
+                               fq_dispatch(dpriv, bio, fqp);
+                       }
+               }
+               FQ_FQP_UNLOCK(fqp);
+       }
        return;
 }
 
 
+static void
+fq_flush(struct disk *dp, struct bio *bio)
+{
+       /* we don't do anything here */
+}
+
+
 static void
 fq_cancel(struct disk *dp)
 {
        struct dsched_fq_dpriv  *dpriv;
-       struct dsched_fq_priv   *fqp, *fqp2;
-       struct bio *bio, *bio2;
 
        dpriv = dsched_get_disk_priv(dp);
        KKASSERT(dpriv != NULL);
@@ -191,19 +222,7 @@ fq_cancel(struct disk *dp)
         * good thing we have a list of fqps per disk dpriv.
         */
        FQ_DPRIV_LOCK(dpriv);
-       TAILQ_FOREACH_MUTABLE(fqp, &dpriv->fq_priv_list, dlink, fqp2) {
-               if (fqp->qlength == 0)
-                       continue;
-
-               FQ_FQP_LOCK(fqp);
-               TAILQ_FOREACH_MUTABLE(bio, &fqp->queue, link, bio2) {
-                       TAILQ_REMOVE(&fqp->queue, bio, link);
-                       --fqp->qlength;
-                       dsched_cancel_bio(bio);
-                       atomic_add_int(&fq_stats.cancelled, 1);
-               }
-               FQ_FQP_UNLOCK(fqp);
-       }
+       fq_drain(dpriv, FQ_DRAIN_CANCEL);
        FQ_DPRIV_UNLOCK(dpriv);
 }
 
@@ -219,6 +238,10 @@ fq_queue(struct disk *dp, struct bio *obio)
        int count;
        int max_tp, transactions;
 
+       /* We don't handle flushes, let dsched dispatch them */
+       if (__predict_false(obio->bio_buf->b_cmd == BUF_CMD_FLUSH))
+               return (EINVAL);
+
        /* get fqmp and fqp */
        fqmp = dsched_get_buf_priv(obio->bio_buf);
 
@@ -231,9 +254,9 @@ fq_queue(struct disk *dp, struct bio *obio)
        KKASSERT(fqmp != NULL);
 #endif
        if (fqmp == NULL) {
+               /* We don't handle this case, let dsched dispatch */
                atomic_add_int(&fq_stats.no_fqmp, 1);
-               dsched_strategy_raw(dp, obio);
-               return 0;
+               return (EINVAL);
        }
 
 
@@ -259,12 +282,9 @@ fq_queue(struct disk *dp, struct bio *obio)
 
        /* XXX: probably rather pointless doing this atomically */
        max_tp = atomic_fetchadd_int(&fqp->max_tp, 0);
-#if 0
-       transactions = atomic_fetchadd_int(&fqp->transactions, 0);
-#endif
        transactions = atomic_fetchadd_int(&fqp->issued, 0);
 
-       /* | No rate limiting || Hasn't reached limit rate |     */
+       /* | No rate limiting || Hasn't reached limit rate | */
        if ((max_tp == 0) || (transactions < max_tp)) {
                /*
                 * Process pending bios from previous _queue() actions that
@@ -397,7 +417,7 @@ fq_dispatch(struct dsched_fq_dpriv *dpriv, struct bio *bio,
        struct timeval tv;
 
        if (dpriv->idle) {
-               getmicrotime(&tv);              
+               getmicrotime(&tv);
                atomic_add_int(&dpriv->idle_time,
                    (int)(1000000*((tv.tv_sec - dpriv->start_idle.tv_sec)) +
                    (tv.tv_usec - dpriv->start_idle.tv_usec)));
index 840f2de..341ed19 100644 (file)
@@ -75,7 +75,7 @@ fq_new_buf(struct buf *bp)
 {
        struct dsched_fq_mpriv  *fqmp = NULL;
 
-       if (curproc != NULL /* XXX: is it -1 or is it 0? */) {
+       if (curproc != NULL) {
                fqmp = dsched_get_proc_priv(curproc);
        } else {
                /* This is a kernel thread, so no proc info is available */