Date: Wed, 20 Feb 2002 20:35:00 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: Terry Lambert <tlambert2@mindspring.com> Cc: Greg Lehey <grog@FreeBSD.ORG>, Jake Burkholder <jake@locore.ca>, "David O'Brien" <obrien@FreeBSD.ORG>, current@FreeBSD.ORG Subject: Re: Patch to improve mutex collision performance Message-ID: <200202210435.g1L4Z0H92642@apollo.backplane.com> References: <200202181912.g1IJCGK32122@apollo.backplane.com> <20020218114326.A98974@dragon.nuxi.com> <200202181951.g1IJpip33604@apollo.backplane.com> <20020218153807.E96115@locore.ca> <20020221111915.N65817@wantadilla.lemis.com> <200202210146.g1L1kqg91511@apollo.backplane.com> <3C746FC1.897E759C@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
:Matthew Dillon wrote:
:> I'm not interested in using P4. I think it's a mistake. That is, I
:> think it is being severely overused. At the very least it is preventing
:> me from comitting simple things to -current because as far as I can tell
:> when you add up the junk sitting in P4 it touches almost every file
:> in the kernel tree. Everything I've tried to work on has some hack
:> sitting in P4 somewhere that somebody hasn't committed.
:
:By the same token, you have also stated:
:
:] Well... try again. If something ought to be compatible in a piecemeal
:] commit and isn't, then something unexpected happened and you need to
:] find out what it was. Doing a full-on commit for something like the
:] proc lock patch is far too dangerous. It's just too large a patch set
:] and we know from experience (cam, softupdates, etc...) that having a
:] small handful of people testing a large private patch is not going to
:] find all the bugs.
:
:How do you reconcile these divergent points of view?
These are not divergent points of view. I am saying quite clearly that
the ucred code and proc-locking code can be committed in a piecemeal
fashion. In fact, good chunk of the proc locking code already has been
with no ill effect on the tree - it might not be completely operational
due to Giant, but just comitting it tests a great deal of infrastructure
including potential lock order reversals and mismatched locks. That's
the whole point of doing things this way.
:What large scale changes are "OK", and what large scale changes
:are "too dangerous"?
The best example I can think of is Julian's initial KSE commit, where
he rearranged the proc structure. The rearrangement touched a huge
number of files and the commit had to be made all at once, not file by
file, due to the structural changes.
:Do you have a set of rules, that would let us look at a patch
:set and instantly decide which of these two categories the
:code fell into?
:
:I'm not trying to be a jerk, but not everything can be broken
:down into 1 line commits, and not everything can be broken down
:into 8 line commits, or 64 line commits, or 512 line commits,
:etc. (if you'll forgive my proof by induction).
:
:-- Terry
This is getting a bit absurd. I am arguing a general principle here,
you are not contributing anything by stretching it into an irrational
statement. One line commits? Oh come on!
I will say that the work *I* am doing on -current is mostly piecemeal
in nature. I even expect the VM locking to wind up being piecemeal.
Everything I have posted to date has been piecemeal. For example,
the ucred patchset I posted does not patch all the ucred functions,
it just patched the read-only functions. But as a side effect that
gave me a basis to track down the other uses of Giant in the general
syscall path. That was a good demarkation point for me. It is by
no means the end... it is, as I have said, piecemeal.
The result? I was able to immediately note the use of Giant in
trap.c (the ucred clearing code) as well as its use in userret(),
plus I could test a few of the ucred functions like getuid() and test
the Giant instrumentation.
So you see, a piecemeal commit can have a great ability to move the
development process forward. When you spend weeks or months putting
together an UBER-patch you do not get any of that... it's all delayed
and when the patch finally goes in people wind up spending a lot of
time testing the patch itself rather then working on the ramification
of what the patch allows you to move on to.
-Matt
Matthew Dillon
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200202210435.g1L4Z0H92642>