Date: Tue, 19 Feb 2002 10:04:43 -0500 (EST) From: Robert Watson <rwatson@FreeBSD.ORG> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: arch@FreeBSD.ORG, jhb@FreeBSD.ORG Subject: Re: John Balwdin's proc-locking patch Message-ID: <Pine.NEB.3.96L.1020219074843.69361V-100000@fledge.watson.org> In-Reply-To: <200202190626.g1J6QhG58642@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Thanks for the technical comments -- I'm sure John will appreciate them once he's connected again. I'll respond briefly to the more philosophical comments. On Mon, 18 Feb 2002, Matthew Dillon wrote: > > * Complexity in some of the ucred patches. For example, in > kern/kern_descrip.c. The patch itself may be fine, but I would > never want to see this committed at the same time everything else > is. I'm not sure that was his intent. I'd imagine he'd bring it in in phases, much the same as anyone would with a large and complex patch. That said, apparently he's been running it on several i386, Alpha, etc, boxes for the past couple of weeks as burn-in. I'd like to see specific components of it brought in earlier, including the ucred components, and signal-related work, for the obvious reasons. > * Lack of use of Giant wrappers, such as in getuid() (though that > is a fairly simple routine). The whole point of using Giant > wrappers is to allow piecemeal commits which are able to test > subsystem mutexes (e.g. with Witness turned on) without having > to let go of Giant, thus keeping the system stable for other > developers until all the piecemeal commits have been done and > to allow the committer to test turning on and off Giant for a > subsytem dynamically, at run-time. As I've said already, I'm fine with you going ahead and committing the instrumented Giant locking. It will make the process less painful for all involved. > General comments: This patch is big, 310 KBytes, and touches a huge > number of files. There are patches for half a dozen subsystems here. > Now I can see something like Julian's first KSE patch needing to be > separated out and committed all in one go because we have no other > choice, but 85% of this particular patch by my reading could have been > committed directly to the main tree in a piecemeal fashion without > harming -current in the least. Probably true, and we should encourage John to do so in the future. That said, you need to understand that this presentation of the patch was presumably not done in the form he would consider ideal, it was done so as to provide broad exposure while he is briefly unavailable. Part of the benefit to John's approach is that he was able to get a whole system view, understanding how locking interacted with the system at all relevant points, rather than selecting a locking strategy and discovering the implications gradually, possibly having to change it later. As a result, he appears to have a tree which is pretty much correctly locked, with substantial testing. > It is totally unecessary to accumulate so many easily-made-incremental > changes off-tree and, in fact, I believe it to be detrimental to the > project. It locks up large areas of code for long periods of time > and creates both a synchronization hassle (even with P4) and a > debugging nightmare when finally committed due to the sheer number > of changes involved. I agree that he should move things in more agressively; that doesn't invalidate the work he has done, however. That said, the technique of maintaining work in p4 and gradually migrating it into the main tree can be a very effective one. It allows you to use a lighter weight committing process to store works-in-progress without interfering with the main tree. For example, in the TrustedBSD branches, we can maintain version control for things we're not sure we even want to commit, but do want to experiment with. We can get version control from an early point in the work, where fundamental assumptions are changing. For example, we're doing a lot of work on the TrustedBSD branches that we'll only migrate in slowly, and in chunks, at a later date. We are still getting a grasp of the full implications of the work, and it would be premature to bring much of it in when we know there will be substantial changes. > This is not a good development philosophy. I'm sorry, it just isn't. > And here I am not blaming John so much as I am blaming the whole > P4 philosophy. I may commit things faster, but I commit much smaller, > more easily tested (and instrumented where necessary) chunks. Forward > progress is made steadily. The commits go in faster because it takes > far less time to test each one and if a problem arises, it is usually > pretty damn obvious what piece is to blame. Not all work can be done quickly and in small commits. Sometimes it's necessary to experiment, or work over a longer period of time. That's one reason why KSE has been done so effectively in a branch: having it in p4 provides more opportunities for collaboration, early review, etc. Julian could get feedback on his work during the development process, rather than as it hit the main tree. Using P4 allowed him to track the main tree efficiently while he did this--something that CVS has always made very difficult. > In our entire history there are only a handful of things that could > be said to legitimately require a separate branch to develop. I > can only think of two right off the bat: CAM and KSEs. Part of that is that we haven't had effective tools to do that in the past, and that we're still learning how to use P4 as a tool. That doesn't invalidate the model: in fact, I'd say that CAM and KSE both provide a strong rationale to use the model, due to their success. For a variety of reasons, less work has been done on SMPng than originally planned: you can blame the economy, specific business decisions, lack of time where time was expected, etc, etc. However, it can still have a structured and organized development process. A quick glance at the SMPng web page suggests that we have exactly that: http://www.FreeBSD.org/smp/ > In anycase, the areas of the patch related to what I was testing > are almost identical, except I include instrumented Giant code > and John simply removes Giant entirely. I prefer my code, frankly. > I do not think it is a good idea to remove Giant entirely for even > the simpler ucred/proc-related system calls at this time. > > That is my position. Then commit your instrumented versions of the giant locking. I specifically excluded that from the list of changes I asked you not to commit, precisely because I agree with many if not all of the technical points you've made. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1020219074843.69361V-100000>