Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Feb 1999 08:02:50 -0800
From:      Kirk McKusick <mckusick@McKusick.COM>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        Julian Elischer <julian@whistle.com>, Jake <jake@checker.org>, Don Lewis <Don.Lewis@tsc.tdk.com>, current@freebsd.org, Jeffrey Hsu <hsu@freebsd.org>
Subject:   Re: softupdate panic, anyone seen this? (fwd) 
Message-ID:  <199902191602.IAA28828@flamingo.McKusick.COM>
In-Reply-To: Your message of "Fri, 19 Feb 1999 02:43:20 PST." <199902191043.CAA31641@apollo.backplane.com> 

next in thread | previous in thread | raw e-mail | index | archive | help
Jeffrey Hsu and I just came to the same conclusion about the splbio
additions earlier this week. I had assumed that Jeffrey had put in
these changes already. Anyway, the two of you need to coordinate
getting the changes put in so that you do not collide.

	Kirk McKusick

=-=-=-=-=-=-=-=

Date: Fri, 19 Feb 1999 02:43:20 -0800 (PST)
From: Matthew Dillon <dillon@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>

    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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199902191602.IAA28828>