Date: Mon, 26 Jun 95 17:30:27 MDT From: terry@cs.weber.edu (Terry Lambert) To: peter@haywire.DIALix.COM (Peter Wemm) Cc: current@freebsd.org, sos@freebsd.org Subject: Re: bug in Linux^H^H^H^H^HDoom Emulator Message-ID: <9506262330.AA00700@cs.weber.edu> In-Reply-To: <Pine.SV4.3.91.950627024851.12512F-100000@haywire.DIALix.COM> from "Peter Wemm" at Jun 27, 95 03:16:58 am
next in thread | previous in thread | raw e-mail | index | archive | help
> > On Mon, 26 Jun 1995, Terry Lambert wrote: > > > > vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv > > DO NOT COMMIT THIS WITHOUT FURTHER CHANGES! > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Seconded! :-) [ ... patches dealing with error returns ... ] > Note that I've changed two "return -1"'s to "return ENOEXEC" - is that > correct? The code appears to be designed to return an errno, and -1 > doesn't appear right at first glance... I can't comment without more source... you'd have to exercise the condition and see if you get what you expected as an error without a full code path. My impression is that -1 is probably correct. > Next question: should those be VOP_UNLOCK's (like in kern_exec.c) or > vput() or vrele()? OK, this whole discussion is starting to become an argument for single entry/single exit of functions. 8-). Again, without more context (like the expected lock state in/out of the locking function) I can't tell at a casual glance whether or not the the count is bumped. Actually, I expect that it is not, though that could be a problem for the SMP work. > I think it probably needs a vput() instead of the VOP_UNLOCK() for all > the cases before the error return, otherwise the reference counts are > going to constantly increase. I suspect that after the VOP_UNLOCK (before > the vm_mmap), all error returns probably need a vrele() first to clear > the reference left by namei(). Is that about right? (the patch above > does not do this - that's why it should not be committed). Ugh. Looks like someone needs to run through the VFS and UFS code and clean up entry/exit state information. The whole FS source base is looking uglier and uglier. 8-(. > Comments? [Now, is this version going to eat my filesystem? Or just use > up all my vnodes? :-)] The net effect is that the FS will not unmount after a failed reference if in fact the reference count is incremented by the lookup. Incrementing it at the lookup is the correct thing to do for SMP (in case of real or effective kernel preemption) to prevent the thing from being incorrectly discarded by one processor in the midst of use by another. The question is if this is correct for SMP or preemption safe or simply unsafe for reentry -- in which case it's not bumping the count. You need to go into the routine that returns the locked node to be sure. Terry Lambert terry@cs.weber.edu --- Any opinions in this posting are my own and not those of my present or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9506262330.AA00700>