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>