Subject: Re: CVS commit: src/sys
To: Darren Reed <darrenr@reed.wattle.id.au>
From: Jason Thorpe <thorpej@wasabisystems.com>
List: tech-kern
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 <thorpej@wasabisystems.com>