- Make 'struct buf *buf' private to vfs_bio.c. Having a global variable

'buf' is inconvenient and has lead me to some irritating to discover
   bugs over the years.  It also makes it more challenging to refactor
   the buf allocation system.
 - Move swbuf and declare it as an extern in vfs_bio.c.  This is still
   not perfect but better than it was before.
 - Eliminate the unused ffs function that relied on knowledge of the buf
   array.
 - Move the shutdown code that iterates over the buf array into vfs_bio.c.

Reviewed by:	kib
Sponsored by:	EMC / Isilon Storage Division

sys/kern/kern_shutdown.c

@@ -275,24 +275,13 @@ doadump(boolean_t textdump)
 	return (error);
 }
 
-static int
-isbufbusy(struct buf *bp)
-{
-	if (((bp->b_flags & (B_INVAL | B_PERSISTENT)) == 0 &&
-	    BUF_ISLOCKED(bp)) ||
-	    ((bp->b_flags & (B_DELWRI | B_INVAL)) == B_DELWRI))
-		return (1);
-	return (0);
-}
-
 /*
  * Shutdown the system cleanly to prepare for reboot, halt, or power off.
  */
 void
 kern_reboot(int howto)
 {
-	static int first_buf_printf = 1;
-	static int waittime = -1;
+	static int once = 0;
 
 #if defined(SMP)
 	/*
@@ -321,116 +310,9 @@ kern_reboot(int howto)
 	/* 
 	 * Now sync filesystems
 	 */
-	if (!cold && (howto & RB_NOSYNC) == 0 && waittime < 0) {
-		register struct buf *bp;
-		int iter, nbusy, pbusy;
-#ifndef PREEMPTION
-		int subiter;
-#endif
-
-		waittime = 0;
-
-		wdog_kern_pat(WD_LASTVAL);
-		sys_sync(curthread, NULL);
-
-		/*
-		 * With soft updates, some buffers that are
-		 * written will be remarked as dirty until other
-		 * buffers are written.
-		 */
-		for (iter = pbusy = 0; iter < 20; iter++) {
-			nbusy = 0;
-			for (bp = &buf[nbuf]; --bp >= buf; )
-				if (isbufbusy(bp))
-					nbusy++;
-			if (nbusy == 0) {
-				if (first_buf_printf)
-					printf("All buffers synced.");
-				break;
-			}
-			if (first_buf_printf) {
-				printf("Syncing disks, buffers remaining... ");
-				first_buf_printf = 0;
-			}
-			printf("%d ", nbusy);
-			if (nbusy < pbusy)
-				iter = 0;
-			pbusy = nbusy;
-
-			wdog_kern_pat(WD_LASTVAL);
-			sys_sync(curthread, NULL);
-
-#ifdef PREEMPTION
-			/*
-			 * Drop Giant and spin for a while to allow
-			 * interrupt threads to run.
-			 */
-			DROP_GIANT();
-			DELAY(50000 * iter);
-			PICKUP_GIANT();
-#else
-			/*
-			 * Drop Giant and context switch several times to
-			 * allow interrupt threads to run.
-			 */
-			DROP_GIANT();
-			for (subiter = 0; subiter < 50 * iter; subiter++) {
-				thread_lock(curthread);
-				mi_switch(SW_VOL, NULL);
-				thread_unlock(curthread);
-				DELAY(1000);
-			}
-			PICKUP_GIANT();
-#endif
-		}
-		printf("\n");
-		/*
-		 * Count only busy local buffers to prevent forcing 
-		 * a fsck if we're just a client of a wedged NFS server
-		 */
-		nbusy = 0;
-		for (bp = &buf[nbuf]; --bp >= buf; ) {
-			if (isbufbusy(bp)) {
-#if 0
-/* XXX: This is bogus.  We should probably have a BO_REMOTE flag instead */
-				if (bp->b_dev == NULL) {
-					TAILQ_REMOVE(&mountlist,
-					    bp->b_vp->v_mount, mnt_list);
-					continue;
-				}
-#endif
-				nbusy++;
-				if (show_busybufs > 0) {
-					printf(
-	    "%d: buf:%p, vnode:%p, flags:%0x, blkno:%jd, lblkno:%jd, buflock:",
-					    nbusy, bp, bp->b_vp, bp->b_flags,
-					    (intmax_t)bp->b_blkno,
-					    (intmax_t)bp->b_lblkno);
-					BUF_LOCKPRINTINFO(bp);
-					if (show_busybufs > 1)
-						vn_printf(bp->b_vp,
-						    "vnode content: ");
-				}
-			}
-		}
-		if (nbusy) {
-			/*
-			 * Failed to sync all blocks. Indicate this and don't
-			 * unmount filesystems (thus forcing an fsck on reboot).
-			 */
-			printf("Giving up on %d buffers\n", nbusy);
-			DELAY(5000000);	/* 5 seconds */
-		} else {
-			if (!first_buf_printf)
-				printf("Final sync complete\n");
-			/*
-			 * Unmount filesystems
-			 */
-			if (panicstr == 0)
-				vfs_unmountall();
-		}
-		swapoff_all();
-		DELAY(100000);		/* wait for console output to finish */
+	if (!cold && (howto & RB_NOSYNC) == 0 && once == 0) {
+		once = 1;
+		bufshutdown(show_busybufs);
 	}
 
 	print_uptime();

sys/kern/subr_param.c

@@ -138,13 +138,6 @@ SYSCTL_PROC(_kern, OID_AUTO, vm_guest, CTLFLAG_RD | CTLTYPE_STRING,
     NULL, 0, sysctl_kern_vm_guest, "A",
     "Virtual machine guest detected?");
 
-/*
- * These have to be allocated somewhere; allocating
- * them here forces loader errors if this file is omitted
- * (if they've been externed everywhere else; hah!).
- */
-struct	buf *swbuf;
-
 /*
  * The elements of this array are ordered based upon the values of the
  * corresponding enum VM_GUEST members.

sys/kern/vfs_bio.c

@@ -64,9 +64,11 @@ __FBSDID("$FreeBSD$");
 #include <sys/resourcevar.h>
 #include <sys/rwlock.h>
 #include <sys/sysctl.h>
+#include <sys/sysproto.h>
 #include <sys/vmem.h>
 #include <sys/vmmeter.h>
 #include <sys/vnode.h>
+#include <sys/watchdog.h>
 #include <geom/geom.h>
 #include <vm/vm.h>
 #include <vm/vm_param.h>
@@ -76,6 +78,7 @@ __FBSDID("$FreeBSD$");
 #include <vm/vm_object.h>
 #include <vm/vm_extern.h>
 #include <vm/vm_map.h>
+#include <vm/swap_pager.h>
 #include "opt_compat.h"
 #include "opt_swap.h"
 
@@ -91,11 +94,8 @@ struct	buf_ops buf_ops_bio = {
 	.bop_bdflush	=	bufbdflush,
 };
 
-/*
- * XXX buf is global because kern_shutdown.c and ffs_checkoverlap has
- * carnal knowledge of buffers.  This knowledge should be moved to vfs_bio.c.
- */
-struct buf *buf;		/* buffer header pool */
+static struct buf *buf;		/* buffer header pool */
+extern struct buf *swbuf;	/* Swap buffer header pool. */
 caddr_t unmapped_buf;
 
 /* Used below and for softdep flushing threads in ufs/ffs/ffs_softdep.c */
@@ -958,6 +958,134 @@ vfs_buf_check_mapped(struct buf *bp)
 	KASSERT(bp->b_data < unmapped_buf || bp->b_data > unmapped_buf +
 	    MAXPHYS, ("b_data + b_offset unmapped %p", bp));
 }
+static int
+isbufbusy(struct buf *bp)
+{
+	if (((bp->b_flags & (B_INVAL | B_PERSISTENT)) == 0 &&
+	    BUF_ISLOCKED(bp)) ||
+	    ((bp->b_flags & (B_DELWRI | B_INVAL)) == B_DELWRI))
+		return (1);
+	return (0);
+}
+
+/*
+ * Shutdown the system cleanly to prepare for reboot, halt, or power off.
+ */
+void
+bufshutdown(int show_busybufs)
+{
+	static int first_buf_printf = 1;
+	struct buf *bp;
+	int iter, nbusy, pbusy;
+#ifndef PREEMPTION
+	int subiter;
+#endif
+
+	/* 
+	 * Sync filesystems for shutdown
+	 */
+	wdog_kern_pat(WD_LASTVAL);
+	sys_sync(curthread, NULL);
+
+	/*
+	 * With soft updates, some buffers that are
+	 * written will be remarked as dirty until other
+	 * buffers are written.
+	 */
+	for (iter = pbusy = 0; iter < 20; iter++) {
+		nbusy = 0;
+		for (bp = &buf[nbuf]; --bp >= buf; )
+			if (isbufbusy(bp))
+				nbusy++;
+		if (nbusy == 0) {
+			if (first_buf_printf)
+				printf("All buffers synced.");
+			break;
+		}
+		if (first_buf_printf) {
+			printf("Syncing disks, buffers remaining... ");
+			first_buf_printf = 0;
+		}
+		printf("%d ", nbusy);
+		if (nbusy < pbusy)
+			iter = 0;
+		pbusy = nbusy;
+
+		wdog_kern_pat(WD_LASTVAL);
+		sys_sync(curthread, NULL);
+
+#ifdef PREEMPTION
+		/*
+		 * Drop Giant and spin for a while to allow
+		 * interrupt threads to run.
+		 */
+		DROP_GIANT();
+		DELAY(50000 * iter);
+		PICKUP_GIANT();
+#else
+		/*
+		 * Drop Giant and context switch several times to
+		 * allow interrupt threads to run.
+		 */
+		DROP_GIANT();
+		for (subiter = 0; subiter < 50 * iter; subiter++) {
+			thread_lock(curthread);
+			mi_switch(SW_VOL, NULL);
+			thread_unlock(curthread);
+			DELAY(1000);
+		}
+		PICKUP_GIANT();
+#endif
+	}
+	printf("\n");
+	/*
+	 * Count only busy local buffers to prevent forcing 
+	 * a fsck if we're just a client of a wedged NFS server
+	 */
+	nbusy = 0;
+	for (bp = &buf[nbuf]; --bp >= buf; ) {
+		if (isbufbusy(bp)) {
+#if 0
+/* XXX: This is bogus.  We should probably have a BO_REMOTE flag instead */
+			if (bp->b_dev == NULL) {
+				TAILQ_REMOVE(&mountlist,
+				    bp->b_vp->v_mount, mnt_list);
+				continue;
+			}
+#endif
+			nbusy++;
+			if (show_busybufs > 0) {
+				printf(
+	    "%d: buf:%p, vnode:%p, flags:%0x, blkno:%jd, lblkno:%jd, buflock:",
+				    nbusy, bp, bp->b_vp, bp->b_flags,
+				    (intmax_t)bp->b_blkno,
+				    (intmax_t)bp->b_lblkno);
+				BUF_LOCKPRINTINFO(bp);
+				if (show_busybufs > 1)
+					vn_printf(bp->b_vp,
+					    "vnode content: ");
+			}
+		}
+	}
+	if (nbusy) {
+		/*
+		 * Failed to sync all blocks. Indicate this and don't
+		 * unmount filesystems (thus forcing an fsck on reboot).
+		 */
+		printf("Giving up on %d buffers\n", nbusy);
+		DELAY(5000000);	/* 5 seconds */
+	} else {
+		if (!first_buf_printf)
+			printf("Final sync complete\n");
+		/*
+		 * Unmount filesystems
+		 */
+		if (panicstr == 0)
+			vfs_unmountall();
+	}
+	swapoff_all();
+	DELAY(100000);		/* wait for console output to finish */
+}
 
 static inline void
 vfs_buf_check_unmapped(struct buf *bp)

sys/sys/buf.h

@@ -465,8 +465,6 @@ extern int	dirtybufthresh;
 extern int	bdwriteskip;
 extern int	dirtybufferflushes;
 extern int	altbufferflushes;
-extern struct	buf *buf;		/* The buffer headers. */
-extern struct	buf *swbuf;		/* Swap I/O buffer headers. */
 extern int	nswbuf;			/* Number of swap I/O buffer headers. */
 extern int	cluster_pbuf_freecnt;	/* Number of pbufs for clusters */
 extern int	vnode_pbuf_freecnt;	/* Number of pbufs for vnode pager */
@@ -485,6 +483,7 @@ void	runningbufwakeup(struct buf *);
 void	waitrunningbufspace(void);
 caddr_t	kern_vfs_bio_buffer_alloc(caddr_t v, long physmem_est);
 void	bufinit(void);
+void	bufshutdown(int);
 void	bdata2bio(struct buf *bp, struct bio *bip);
 void	bwillwrite(void);
 int	buf_dirty_count_severe(void);

sys/ufs/ffs/ffs_subr.c

@@ -55,10 +55,6 @@ __FBSDID("$FreeBSD$");
 #include <ufs/ffs/ffs_extern.h>
 #include <ufs/ffs/fs.h>
 
-#ifdef KDB
-void	ffs_checkoverlap(struct buf *, struct inode *);
-#endif
-
 /*
  * Return buffer with the contents of block "offset" from the beginning of
  * directory "ip".  If "res" is non-zero, fill it in with a pointer to the
@@ -165,37 +161,6 @@ ffs_fragacct(fs, fragmap, fraglist, cnt)
 	}
 }
 
-#ifdef KDB
-void
-ffs_checkoverlap(bp, ip)
-	struct buf *bp;
-	struct inode *ip;
-{
-	struct buf *ebp, *ep;
-	ufs2_daddr_t start, last;
-	struct vnode *vp;
-
-	ebp = &buf[nbuf];
-	start = bp->b_blkno;
-	last = start + btodb(bp->b_bcount) - 1;
-	for (ep = buf; ep < ebp; ep++) {
-		if (ep == bp || (ep->b_flags & B_INVAL) ||
-		    ep->b_vp == NULLVP)
-			continue;
-		vp = ip->i_devvp;
-		/* look for overlap */
-		if (ep->b_bcount == 0 || ep->b_blkno > last ||
-		    ep->b_blkno + btodb(ep->b_bcount) <= start)
-			continue;
-		vprint("Disk overlap", vp);
-		printf("\tstart %jd, end %jd overlap start %jd, end %jd\n",
-		    (intmax_t)start, (intmax_t)last, (intmax_t)ep->b_blkno,
-		    (intmax_t)(ep->b_blkno + btodb(ep->b_bcount) - 1));
-		panic("ffs_checkoverlap: Disk buffer overlap");
-	}
-}
-#endif /* KDB */
-
 /*
  * block operations
  *

sys/vm/vm_pager.c

@@ -86,6 +86,8 @@ __FBSDID("$FreeBSD$");
 
 int cluster_pbuf_freecnt = -1;	/* unlimited to begin with */
 
+struct buf *swbuf;
+
 static int dead_pager_getpages(vm_object_t, vm_page_t *, int, int);
 static vm_object_t dead_pager_alloc(void *, vm_ooffset_t, vm_prot_t,
     vm_ooffset_t, struct ucred *);