Date: Fri, 18 Apr 1997 13:52:36 +0900 (JST) From: Michael Hancock <michaelh@cet.co.jp> To: Darren Reed <darrenr@cyber.com.au> Cc: kato@freefall.freebsd.org, current@freebsd.org Subject: Consistency checks (was Re: cvs commit: src/sys/miscfs/union union_vnops.c) Message-ID: <Pine.SV4.3.95.970418132251.473A-100000@parkplace.cet.co.jp> In-Reply-To: <199704160851.SAA09685@plum.cyber.com.au>
next in thread | previous in thread | raw e-mail | index | archive | help
[moved to current] On Wed, 16 Apr 1997, Darren Reed wrote: > In some mail I received from Michael Hancock, sie wrote > > > > I saw that you found the real fix later. Cool. > > > > Regarding the below, I think it's better to panic in this case. Making > > things robust often works against making it work correctly. Consistency > > checks that result in panics are there to help you find problems. Working > > around consistency checks usually results in crufty code. > > A system that is up is a good thing. > > If a systems admin. can crash it by typing a mount command (for example) > incorrectly, that is bad (IMHO). I see what you're getting at but I'd > prefer it to return a nasty error. > You guys both make valid point and I like to point out a couple of things: a) You're working in current and most administrators are smart enough to know they should be using some stable. b) Unionfs is widely known to be broken and it is not a central subsystem that everyone depends upon yet. People experimenting with it should get into a habit of doing a sync before using it in current. c) In a release, if it is still broken the mount_union command can be disabled so that it doesn't trigger a kernel panic. Kato-san is on about using good judgement case by case and I'd like to offer some guidelines that are helpful when in doubt about a micro-decision, development strategy, or whatever programming habit you decide to use. Pick out some external software system qualities and order them by importance to you and preferrably most others. It's good to know what others think are important anyway. I like to look at the following dimensions in the following order or priority: 1) Correctness - It does what it was designed to do. 2) Performance - This matters more than you think. 3) Robustness - This matters to a lot of users, but it can work against 1 and 2. The solution I provided above satisfies these 3 qualities in the following ways: 1) The consistency checks will help you find bugs sooner and finding and fixing bugs early leads to higher quality code. 2) Performance is enhanced because you don't have a bunch of 'if' statements written solely for the purpose of working around a bug. 3) The code will be more readable helping you to find the bug. You don't need to wade through a bunch of work arounds. 4) In the release code, if the bug is still not found. The external user land mount_union is disabled, preventing users from crashing the system. This method of coding in robustness will look more professional to the end user than a hang and it doesn't interfere with internal debugging techniques. Regards, Mike Hancock > > > kato 97/04/15 05:57:00 > > > > > > Modified: sys/miscfs/union union_vnops.c > > > Log: > > > Quick-hack to avoid `lock against myself' panic. It is not the real > > > fix! > > > > > > The ufs_link() assumes that vnode is not unlocked and tries to lock it > > > in certain case. Because union_link calls VOP_LINK after locking vnode, > > > vn_lock in ufs_link causes above panic. > > > > > > Currently, I don't know the real fix for a locking violation in > > > union_link, but I think it is important to avoid panic. > > > > > > A vnode is unlocked before calling VOP_LINK and is locked after it if > > > the vnode is not union fs. Even though panic went away, the process > > > that access the union fs in which link was made will hang-up. > > > > > > Hang-up can be easily reproduced by following operation: > > > > > > mount -t union a b > > > cd b > > > ln foo bar > > > ls > > > > > > Revision Changes Path > > > 1.24 +15 -2 src/sys/miscfs/union/union_vnops.c > > > > > > > > -- michaelh@cet.co.jp http://www.cet.co.jp CET Inc., Daiichi Kasuya BLDG 8F 2-5-12, Higashi Shinbashi, Minato-ku, Tokyo 105 Japan Tel: +81-3-3437-1761 Fax: +81-3-3437-1766
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.SV4.3.95.970418132251.473A-100000>