This checkin reimplements the io-request priority hack in a way

that works in the new threaded kernel. It was commented out of
the disksort routine earlier this year for the reasons given in
kern/subr_disklabel.c (which is where this code used to reside
before it moved to kern/subr_disk.c):

----------------------------
revision 1.65
date: 2002/04/22 06:53:20;  author: phk;  state: Exp;  lines: +5 -0
Comment out Kirks io-request priority hack until we can do this in a
civilized way which doesn't cause grief.

The problem is that it is not generally safe to cast a "struct bio
*" to a "struct buf *".  Things like ccd, vinum, ata-raid and GEOM
constructs bio's which are not entrails of a struct buf.

Also, curthread may or may not have anything to do with the I/O request
at hand.

The correct solution can either be to tag struct bio's with a
priority derived from the requesting threads nice and have disksort
act on this field, this wouldn't address the "silly-seek syndrome"
where two equal processes bang the diskheads from one edge to the
other of the disk repeatedly.

Alternatively, and probably better: a sleep should be introduced
either at the time the I/O is requested or at the time it is completed
where we can be sure to sleep in the right thread.

The sleep also needs to be in constant timeunits, 1/hz can be practicaly
any sub-second size, at high HZ the current code practically doesn't
do anything.
----------------------------

As suggested in this comment, it is no longer located in the disk sort
routine, but rather now resides in spec_strategy where the disk operations
are being queued by the thread that is associated with the process that
is really requesting the I/O. At that point, the disk queues are not
visible, so the I/O for positively niced processes is always slowed
down whether or not there is other activity on the disk.

On the issue of scaling HZ, I believe that the current scheme is
better than using a fixed quantum of time. As machines and I/O
subsystems get faster, the resolution on the clock also rises.
So, ten years from now we will be slowing things down for shorter
periods of time, but the proportional effect on the system will
be about the same as it is today. So, I view this as a feature
rather than a drawback. Hence this patch sticks with using HZ.

Sponsored by:	DARPA & NAI Labs.
Reviewed by:	Poul-Henning Kamp <phk@critter.freebsd.dk>

3 files changed, 26 insertions, 32 deletions

diff --git a/sys/fs/specfs/spec_vnops.c b/sys/fs/specfs/spec_vnops.c
index 51c2792..9ae87c0 100644
--- a/sys/fs/specfs/spec_vnops.c
+++ b/sys/fs/specfs/spec_vnops.c
@@ -40,6 +40,7 @@
 #include <sys/proc.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <sys/mutex.h>
 #include <sys/conf.h>
 #include <sys/bio.h>
 #include <sys/buf.h>
@@ -493,6 +494,18 @@ loop2:
 }
 
 /*
+ * Mutex to use when delaying niced I/O bound processes in spec_strategy().
+ */
+static struct mtx strategy_mtx;
+static void
+strategy_init(void)
+{
+
+	mtx_init(&strategy_mtx, "strategy", NULL, MTX_DEF);
+}
+SYSINIT(strategy, SI_SUB_DRIVERS, SI_ORDER_MIDDLE, strategy_init, NULL)
+
+/*
  * Just call the device strategy routine
  */
 static int
@@ -507,7 +520,17 @@ spec_strategy(ap)
 	struct mount *mp;
 	int error;
 	struct cdevsw *dsw;
-
+	struct thread *td = curthread;
+	
+	/*
+	 * Slow down disk requests for niced processes.
+	 */
+	if (td && td->td_ksegrp->kg_nice > 0) {
+		mtx_lock(&strategy_mtx);
+		msleep(&strategy_mtx, &strategy_mtx,
+		    PPAUSE | PCATCH | PDROP, "ioslow",
+		    td->td_ksegrp->kg_nice);
+	}
 	bp = ap->a_bp;
 	vp = ap->a_vp;
 	if (bp->b_iocmd == BIO_WRITE) {
diff --git a/sys/kern/subr_disk.c b/sys/kern/subr_disk.c
index c61af99..d6943df 100644
--- a/sys/kern/subr_disk.c
+++ b/sys/kern/subr_disk.c
@@ -479,20 +479,6 @@ disk_err(struct bio *bp, const char *what, int blkdone, int nl)
 		printf("\n");
 }
 
-#ifdef notquite
-/*
- * Mutex to use when delaying niced I/O bound processes in bioq_disksort().
- */
-static struct mtx dksort_mtx;
-static void
-dksort_init(void)
-{
-
-	mtx_init(&dksort_mtx, "dksort", NULL, MTX_DEF);
-}
-SYSINIT(dksort, SI_SUB_DRIVERS, SI_ORDER_MIDDLE, dksort_init, NULL)
-#endif
-
 /*
  * Seek sort for disks.
  *
@@ -517,21 +503,6 @@ bioq_disksort(bioq, bp)
 	struct bio *bn;
 	struct bio *be;
 
-#ifdef notquite
-	struct thread *td = curthread;
-	
-	if (td && td->td_ksegrp->kg_nice > 0) {
-		TAILQ_FOREACH(bn, &bioq->queue, bio_queue)
-			if (BIOTOBUF(bp)->b_vp != BIOTOBUF(bn)->b_vp)
-				break;
-		if (bn != NULL) {
-			mtx_lock(&dksort_mtx);
-			msleep(&dksort_mtx, &dksort_mtx,
-			    PPAUSE | PCATCH | PDROP, "ioslow",
-			    td->td_ksegrp->kg_nice);
-		}
-	}
-#endif
 	if (!atomic_cmpset_int(&bioq->busy, 0, 1))
 		panic("Recursing in bioq_disksort()");
 	be = TAILQ_LAST(&bioq->queue, bio_queue);
diff --git a/sys/ufs/ffs/ffs_snapshot.c b/sys/ufs/ffs/ffs_snapshot.c
index 752822d..b22b193 100644
--- a/sys/ufs/ffs/ffs_snapshot.c
+++ b/sys/ufs/ffs/ffs_snapshot.c
@@ -493,8 +493,6 @@ out1:
 	 * Resume operation on filesystem.
 	 */
 	vfs_write_resume(vp->v_mount);
-	if (saved_nice > 0)
-		td->td_ksegrp->kg_nice = saved_nice;
 	vn_start_write(NULL, &wrtmp, V_WAIT);
 	if (collectsnapstats && starttime.tv_sec > 0) {
 		nanotime(&endtime);
@@ -592,6 +590,8 @@ done:
 	free(copy_fs->fs_csp, M_UFSMNT);
 	bawrite(sbp);
 out:
+	if (saved_nice > 0)
+		td->td_ksegrp->kg_nice = saved_nice;
 	if (fs->fs_active != 0) {
 		FREE(fs->fs_active, M_DEVBUF);
 		fs->fs_active = 0;