From owner-freebsd-current Tue Sep 22 06:23:54 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id GAA00439 for freebsd-current-outgoing; Tue, 22 Sep 1998 06:23:54 -0700 (PDT) (envelope-from owner-freebsd-current@FreeBSD.ORG) Received: from gatekeeper.tsc.tdk.com (gatekeeper.tsc.tdk.com [207.113.159.21]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id GAA00429 for ; Tue, 22 Sep 1998 06:23:52 -0700 (PDT) (envelope-from gdonl@tsc.tdk.com) Received: from sunrise.gv.tsc.tdk.com (root@sunrise.gv.tsc.tdk.com [192.168.241.191]) by gatekeeper.tsc.tdk.com (8.8.8/8.8.8) with ESMTP id GAA22628; Tue, 22 Sep 1998 06:23:18 -0700 (PDT) (envelope-from gdonl@tsc.tdk.com) Received: from salsa.gv.tsc.tdk.com (salsa.gv.tsc.tdk.com [192.168.241.194]) by sunrise.gv.tsc.tdk.com (8.8.5/8.8.5) with ESMTP id GAA22816; Tue, 22 Sep 1998 06:23:17 -0700 (PDT) Received: (from gdonl@localhost) by salsa.gv.tsc.tdk.com (8.8.5/8.8.5) id GAA17424; Tue, 22 Sep 1998 06:23:16 -0700 (PDT) From: Don Lewis Message-Id: <199809221323.GAA17424@salsa.gv.tsc.tdk.com> Date: Tue, 22 Sep 1998 06:23:16 -0700 In-Reply-To: Luoqi Chen "Re: Yet another patch to try for softupdates panic" (Sep 21, 7:27pm) X-Mailer: Mail User's Shell (7.2.6 alpha(3) 7/19/95) To: Luoqi Chen , current@FreeBSD.ORG Subject: Re: Yet another patch to try for softupdates panic Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sep 21, 7:27pm, Luoqi Chen wrote: } Subject: Re: Yet another patch to try for softupdates panic } > It looks like this bogus unlocking could also affect vinvalbuf() which } > expects the vnode to be locked. } > } The unlock is not bogus at all. You have to maintain a strict locking order } to avoid deadlocks. For vnodes, this locking order is the directory tree } itself, i.e., you have to lock the parent first, then the child. So if you } hold the lock on the child and intend to lock the parent, you have to } release the lock on child first, then acquire the lock on the parent, and } then reacquire lock the the child. That's also the reason why you can't have } hardlinks to a directory. Yes, I read the comments in the code and understand why the locking must be done that way, and I also understand why this new code is needed when a user process calls fsync() for a file that exists on a filesystem using softupdates. What's bogus is that this new code changes the semantics of VOP_FSYNC(). The fact that VOP_FSYNC() may now unlock the vnode for a while then lock it again may break other parts of the kernel that expect the vnode to remain locked across a call VOP_FSYNC(). This change in semantics caused the directory truncation race that your patch fixed, although admittedly this particular call to VOP_FSYNC() was added to support softupdates. For another example, take a look at vinvalbuf(), which has the following code: /* * Flush out and invalidate all buffers associated with a vnode. * Called with the underlying object locked. */ int vinvalbuf(vp, flags, cred, p, slpflag, slptimeo) [ snip ] if (vp->v_dirtyblkhd.lh_first != NULL) { splx(s); if ((error = VOP_FSYNC(vp, cred, MNT_WAIT, p)) != 0) return (error); s = splbio(); if (vp->v_numoutput > 0 || vp->v_dirtyblkhd.lh_first != NULL) panic("vinvalbuf: dirty bufs"); } splx(s); It sure looks to me like if VOP_FSYNC() called ffs_fsync(), ffs_fsync() would first write all the dirty buffers and then call softdep_fsync(). Softdep_fsync() would unlock the vnode in order to sync the parent directories, and while the vnode was unlocked, another process could grab the vnode and dirty its buffers. Softdep_fsync() would relock the vnode and return. When VOP_FSYNC() returns, we get a nice panic ... You could tweak this call to VOP_FSYNC() to get it to avoid the call to softdep_fsync(), but how many other places in the kernel also need to be fixed? It may be better to only call softdep_fsync() from within the fsync() syscall handler. I don't know that any other users of VOP_FSYNC() need to ensure that the parent directories are pushed to disk. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message