From owner-freebsd-current  Fri Feb 19  2:43:28 1999
Delivered-To: freebsd-current@freebsd.org
Received: from apollo.backplane.com (apollo.backplane.com [209.157.86.2])
	by hub.freebsd.org (Postfix) with ESMTP id 52F5711549
	for <current@FreeBSD.ORG>; Fri, 19 Feb 1999 02:43:21 -0800 (PST)
	(envelope-from dillon@apollo.backplane.com)
Received: (from dillon@localhost)
	by apollo.backplane.com (8.9.3/8.9.1) id CAA31641;
	Fri, 19 Feb 1999 02:43:20 -0800 (PST)
	(envelope-from dillon)
Date: Fri, 19 Feb 1999 02:43:20 -0800 (PST)
From: Matthew Dillon <dillon@apollo.backplane.com>
Message-Id: <199902191043.CAA31641@apollo.backplane.com>
To: Julian Elischer <julian@whistle.com>
Cc: Kirk McKusick <mckusick@McKusick.COM>, Jake <jake@checker.org>,
	Don Lewis <Don.Lewis@tsc.tdk.com>, current@FreeBSD.ORG
Subject: Re: softupdate panic, anyone seen this? (fwd) 
References:  <Pine.BSF.3.95.990218132053.16922A-100000@current1.whistle.com>
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

    This may or may not be related.  In tracking down the sched_sync() 
    panic I found two bugs.  

    First, a couple of places where the worklist was not being protected
    at splbio().  I'm not 100% sure that this is a problem but the code
    is complex enough that it's just too dangerous not to do it.

    Second, a double LIST_REMOVE() was being performed in the case where
    VOP_FSYNC() would fail to sync all the dirty pages.  This can occur
    legally for both NFS and filesystems with SOFTUPDATES set.

    I'd appreciate it if someone could verify the double LIST_REMOVE()
    bug.  vn_syncer_add_to_worklist() already removes the vn from
    the list ( assuming the VONWORKLIST v_flag is set, which it should be
    in this case ).

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

Index: kern/vfs_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.186
diff -u -r1.186 vfs_subr.c
--- vfs_subr.c	1999/02/04 18:25:39	1.186
+++ vfs_subr.c	1999/02/19 10:40:17
@@ -881,10 +881,8 @@
 /*
  * Add an item to the syncer work queue.
  */
-void
-vn_syncer_add_to_worklist(vp, delay)
-	struct vnode *vp;
-	int delay;
+static void
+vn_syncer_add_to_worklist(struct vnode *vp, int delay)
 {
 	int s, slot;
 
@@ -928,7 +926,8 @@
 		starttime = time_second;
 
 		/*
-		 * Push files whose dirty time has expired.
+		 * Push files whose dirty time has expired.  Be careful
+		 * of interrupt race on slp queue.
 		 */
 		s = splbio();
 		slp = &syncer_workitem_pending[syncer_delayno];
@@ -941,16 +940,20 @@
 			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
 			(void) VOP_FSYNC(vp, p->p_ucred, MNT_LAZY, p);
 			VOP_UNLOCK(vp, 0, p);
+			s = splbio();
 			if (LIST_FIRST(slp) == vp) {
 				if (TAILQ_EMPTY(&vp->v_dirtyblkhd) &&
 				    vp->v_type != VBLK)
-					panic("sched_sync: fsync failed");
+					panic("sched_sync: fsync failed vp %p type %d tag %d", vp, vp->v_type, vp->v_tag);
 				/*
 				 * Move ourselves to the back of the sync list.
+				 * vn_syncer_*worklist() will remove and re-add
+				 * the node.
 				 */
-				LIST_REMOVE(vp, v_synclist);
+				/*LIST_REMOVE(vp, v_synclist);*/
 				vn_syncer_add_to_worklist(vp, syncdelay);
 			}
+			splx(s);
 		}
 
 		/*
@@ -2841,6 +2844,8 @@
 
 /*
  * The syncer vnode is no longer needed and is being decommissioned.
+ *
+ * Modifications to the worklist must be protected at splbio().
  */
 static int
 sync_reclaim(ap)
@@ -2849,12 +2854,15 @@
 	} */ *ap;
 {
 	struct vnode *vp = ap->a_vp;
+	int s;
 
+	s = splbio();
 	vp->v_mount->mnt_syncer = NULL;
 	if (vp->v_flag & VONWORKLST) {
 		LIST_REMOVE(vp, v_synclist);
 		vp->v_flag &= ~VONWORKLST;
 	}
+	splx(s);
 
 	return (0);
 }


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message