Date: Tue, 22 Sep 1998 12:05:22 -0400 (EDT) From: Luoqi Chen <luoqi@watermarkgroup.com> To: Don.Lewis@tsc.tdk.com, current@FreeBSD.ORG, luoqi@watermarkgroup.com Cc: mckusick@flamingo.McKusick.COM Subject: Re: Yet another patch to try for softupdates panic Message-ID: <199809221605.MAA01306@lor.watermarkgroup.com>
next in thread | raw e-mail | index | archive | help
> 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 ... > Yes, there's a race here, and also a couple of other places VOP_FSYNC() are called. > 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. > I totally agree. This is the way to go. I'll try this and let you know how it turns out. -lq 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?199809221605.MAA01306>