Re: three kernel patches for review

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:Hi all,
:
:For review by interested parties, attached:
:
:1. kern-ansi.diff:
:   ANSIfy a bunch of function definitions in sys/kern/kern_*.c files.
:   (not contentious, although might cause minor conflicts for other
:    developers holding local modified versions of those files)
:
:2. bpf-comments.diff:
:   Update some comments for bpf_validate() to make them reflect
:   reality.  It doesn't check to make sure that jumps go forward;
:   jumps are unsigned, and thus always go forward.  Also fix a
:   typo in the man page.
:
:3. vm-badram.diff:
:   Port jhb's commit to FreeBSD CURRENT, sys/vm/vm_pageq.c rev 1.15,
:   which allows a list of known-bad memory locations to be specified,
:   which the VM subsystem will not attempt to map.  (I also added an
:   ASSERT_IN_CRIT_SECTION to the function along the way, since the
:   comments note that it should only be called in a critical section.)
:   This is mainly for discussion - would people find this useful?
:   Is there a better way we can accomplish the same thing?  Are there
:   any additional improvement that can be made to it (e.g. allow listing
:   a range of addresses?)
:
:-Chris


: /*
:  * Accounting system call.  Written based on the specification and
:  * previous implementation done by Mark Tinguely.
:+ *
:+ * int acct(struct acct_args {
:+ *		syscallarg(char *) path;
:+ *	    } *uap);
:  */

    Pretty wordy.  I recommend using the comment form that we use for
    other syscalls.  Example:

    /*
     * socket_args(int domain, int type, int protocol)
     */
    int
    socket(struct socket_args *uap)
    ...

    Otherwise, the ANSIfication patch generally looks good, go ahead and
    commit #1 after you clean up that comment.

    On #2 ... looks reasonable.  Commit away!

    On #3 ... that doesn't look so reasonable.  I suppose on a machine with
    huge amounts of memory one might want such a mechanism, but frankly if
    memory is bad (especially if it is ECC'd memory), the only correct
    solution is to replace it.  

    So I would say 'no' on the badram patch, at least for now.

					-Matt
					Matthew Dillon 
					<dillon@xxxxxxxxxxxxx>