Subject: Re: CVS commit: src/sys
To: Darren Reed <firstname.lastname@example.org>
From: Jason Thorpe <email@example.com>
Date: 06/29/2003 12:29:43
On Sunday, June 29, 2003, at 11:35 AM, Darren Reed wrote:
> Hmmm...I replaced, for the most part, use of "proc" with "lwp".
Yes, and a good portion of those appear to have been unnecessary. In
addition to the vget()-and-fallout change I posted about (and just
fixed), you also have changed some operations that are logically a
per-process operation to take an lwp * instead of a proc *.
One such example I've encountered is selrecord(). selrecord() used the
proc * argument only to record that "a process is selecting on this".
It does not care which lwp is selecting, just that "a process" is.
Your change to selrecord() to take an lwp * instead of a proc * only
changed how the "mypid" local variable was assigned in selrecord().
The change to selrecord() bubbles up to affect all callers of
selrecord(), some of which in-turn likely had their arguments changes
in order to (unnecessarily) pass an lwp * to selrecord().
This appears to be the case with ALL device switch entry points. So
far I have not seen any need for those routines to take lwp *'s, other
than, perhaps, to pass an lwp * down to selrecord(), which doesn't even
care about lwp's in the first place.
What this boils down to is that you appear to have made quite a number
of changes unrelated to actually providing LWPIDs in ktrace records.
This artificially inflated the size of your patch, and thus made it
much more difficult for others to review; the larger a diff is, the
easier it is to miss things like the ones I've posted about thus far.
-- Jason R. Thorpe <firstname.lastname@example.org>