Date: Fri, 27 Feb 1998 19:20:08 +0000 (GMT) From: Terry Lambert <tlambert@primenet.com> To: eivind@yes.no (Eivind Eklund) Cc: jlemon@americantv.com, eivind@yes.no, fs@FreeBSD.ORG Subject: Re: syncer / SMP question Message-ID: <199802271920.MAA17452@usr05.primenet.com> In-Reply-To: <19980227190132.53798@follo.net> from "Eivind Eklund" at Feb 27, 98 07:01:32 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> > It appears to be released. The vfs_busy() routine makes a call to > > lockmgr(), which sets a lock on mountpoint and releases the lock on > > the mountlist. > > But... But... What about the fetch of the next-pointer? Looks like > a potential race-condition to me, but this includes some lock-calls I > don't really understand what do (interlocks? What are they?). The placement of the nmp assignment is correct. This allows the nmp to *not* be advanced through several loop iterations, if necessary, apart from the other benefits noted. > > I agree that the lock calls look weird. Perhaps someone could explain > > the purpose of the various locks? It appears that you need to get a lock > > on the mountlist in order to iterate the mountpoints. But why does > > vfs_busy() (which operates on a mountpoint) release your mountlist lock? > > Convenience? > > Looks likely. The functionality is actually only used five places: > sync(), getfsstat(), printlockedvnodes(), sysctl_vnode(), and > nqnfs_lease_updatetime(), all in equal-looking loops. Almost all of the lock code is non-reflexive. If you consider memory allocations as pointer/region locks, then even more of it is screwed (my favorite example here is "namei" allocating the path buffer, and each FS being expected to free it). > Yes, it is probably more code than presently (at least if you count it > as lines of C-code), but it abstract away all the complexity, and make > the interfaces clean. Locking and unlocking at different layers is > IMHO not clean; it 'smells of trouble'. Heh... "Welcome to my world". I'm also in favor of documenting locks held in and out of all functions in the function comments, and making all functions single-entry/single-exit to facilitate state tracking and enforcement in debug kernels (using assertions). > > (Yes, I know that simple_lock is only for SMP, but still...) > > > > As for hoisting up the calculation of nmp, I don't think you can do that, > > since the inner portion releases it's lock on the mountlist, meaning that > > the another processor can change the mountlist out from underneath you, > > invalidating your (saved) nmp. > > Yes, I later noticed. What do you think of the above? I think you are bailing the ocean with a bucket. This whole issue needs a top level design, and then style enforcement, not just a onesey-twosey fix where it's obvious. You will need to drag in the core team for a style change discussion. Terry Lambert terry@lambert.org --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199802271920.MAA17452>