From owner-freebsd-current Mon Jun 26 16:37:30 1995 Return-Path: current-owner Received: (from majordom@localhost) by freefall.cdrom.com (8.6.10/8.6.6) id QAA20803 for current-outgoing; Mon, 26 Jun 1995 16:37:30 -0700 Received: from cs.weber.edu (cs.weber.edu [137.190.16.16]) by freefall.cdrom.com (8.6.10/8.6.6) with SMTP id QAA20795 ; Mon, 26 Jun 1995 16:37:28 -0700 Received: by cs.weber.edu (4.1/SMI-4.1.1) id AA00700; Mon, 26 Jun 95 17:30:28 MDT From: terry@cs.weber.edu (Terry Lambert) Message-Id: <9506262330.AA00700@cs.weber.edu> Subject: Re: bug in Linux^H^H^H^H^HDoom Emulator To: peter@haywire.DIALix.COM (Peter Wemm) Date: Mon, 26 Jun 95 17:30:27 MDT Cc: current@freebsd.org, sos@freebsd.org In-Reply-To: from "Peter Wemm" at Jun 27, 95 03:16:58 am X-Mailer: ELM [version 2.4dev PL52] Sender: current-owner@freebsd.org Precedence: bulk > > 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.