On 28-May-2002 Julian Elischer wrote:
> Now things are moving again. 
> Jonathon Mini and I have cleaned up the patches a bit
> and fixed the more obvious problems..
> from teh point of view of non KSE processes 
> (that would be all of them at the moment) it shuold act the
> same as now. Hopefully, changes are restricted to instruction flows that 
> would only occur in KSE processes.
> I would like to commit this in the next couple of days to give it enough
> time to settle BEFORE USENIX.

Ok, here are the random comments I've come up with:

1) Why gratuitous rename of p_stat to p_state?

2) I object to removing cred_free_thread().  It isn't getting in your
   way and some of us like to test things.

3) Why did you not make the linux process in linux_machdep.c runnable
   before you did a setrunqueue()?  You do know we create it
   stopped at first and then make it runnable after doing fixups
   after fork1(), right?

4) It would be nice if you didn't mix in gratuitous style changes with
   actual content changes such as extra braces in else clause of
   PROCFS_CTRL_WAIT case in procfs_ctl.c

5) The comment by thread_unsuspend() in procfs_ctl.c seems gratuitous,
   that is what one would expect from such a function.

6) In cpu_set_retval() you have:

+cpu_set_retval(struct thread *td, int retval, int aux, int success)
+       struct trapframe *frame;
+       frame = td->td_frame;
+       frame->tf_eax = retval;         /* Child returns zero */
+       frame->tf_edx = aux;            /* I dunno */

You could always ask about that instead of having a I dunno comment. :)
I think that we no longer use 2 return values from syscalls for FreeBSD
syscalls (I know we did for fork1() at one point, possibly still do
so that 4.x libc works ok on 5.x kernel).  Linux does depend on edx being
preserved across a syscall though IIRC.

7) Please follow the established convention for names of members in
   pcb_ext and call 'refcount' 'ext_refccount'.

8) It would be nice if the CURSIG() -> cursig() change were committed
   separately to avoid obfuscating the other diffs.

9) More gratuitous braces as well as gratuituos ()'s and white space
   changes in ithread_schedule() obfuscate the functional diffs.

10) In kern_sig.c, prototypes are declared on one line and near the
   beginning of the file, not in the middle of code (tdsignal).

11) Why is there a whole chunk of code #if 0'd out in kern_sig.c?

12) You lock p_pptr twice (can't do that) before psignal().  Looks
   like you added an extra one along with gratuitous braces and a
   whitespace change.

13) Could you call readjustrunqueue() runq_readjust() to follow the
   other namespaces already in kern_switch.c?  Also, I find it easier
   to read personally.

14) Hmm, I'm not sure why you need TDF_INMSLEEP if a KSE always has
   a spare thread that is replenished first thing when the spare
   thread prepares to do an upcall.

15) The 'um... dunno' comment in abortsleep() is a bit unsettling, do
   you think you could clarify / run some tests to figure out exactly
   what is going on there?

16) I don't think we need the P_SPARE's.

17) I think SINGLE_WAIT/EXIT is slightly more readable than

18) I still don't see what's so hard about LIST_FOREACH() with


John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to