From 3ee00e04e58b9c411500d32ca897e49bfb913596 Mon Sep 17 00:00:00 2001 From: Alex Hornung Date: Thu, 1 Apr 2010 01:09:59 +0000 Subject: [PATCH] dsched_fq - Refactor and clean; handle flushes * 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 | 10 ++--- sys/dsched/fq/dsched_fq_core.c | 38 ++--------------- sys/dsched/fq/dsched_fq_diskops.c | 70 ++++++++++++++++++++----------- sys/dsched/fq/dsched_fq_procops.c | 2 +- 4 files changed, 53 insertions(+), 67 deletions(-) diff --git a/sys/dsched/fq/dsched_fq.h b/sys/dsched/fq/dsched_fq.h index 23117c62b6..ba10f746c1 100644 --- a/sys/dsched/fq/dsched_fq.h +++ b/sys/dsched/fq/dsched_fq.h @@ -86,14 +86,9 @@ #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 */ diff --git a/sys/dsched/fq/dsched_fq_core.c b/sys/dsched/fq/dsched_fq_core.c index b08dc09767..d87d63436e 100644 --- a/sys/dsched/fq/dsched_fq_core.c +++ b/sys/dsched/fq/dsched_fq_core.c @@ -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++) { diff --git a/sys/dsched/fq/dsched_fq_diskops.c b/sys/dsched/fq/dsched_fq_diskops.c index ef2badcf9f..886a9991ef 100644 --- a/sys/dsched/fq/dsched_fq_diskops.c +++ b/sys/dsched/fq/dsched_fq_diskops.c @@ -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))); diff --git a/sys/dsched/fq/dsched_fq_procops.c b/sys/dsched/fq/dsched_fq_procops.c index 840f2de385..341ed199f8 100644 --- a/sys/dsched/fq/dsched_fq_procops.c +++ b/sys/dsched/fq/dsched_fq_procops.c @@ -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 */ -- 2.41.0