Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Mar 2003 14:02:06 +0200
From:      Giorgos Keramidas <keramida@FreeBSD.org>
To:        Tom Rhodes <trhodes@FreeBSD.org>
Cc:        FreeBSD-audit@FreeBSD.org
Subject:   Re: [PATCH] Review Requested
Message-ID:  <20030318120206.GD25064@gothmog.gr>
In-Reply-To: <20030318010433.7e2adea1.trhodes@FreeBSD.org>
References:  <20030318010433.7e2adea1.trhodes@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 2003-03-18 01:04, Tom Rhodes <trhodes@FreeBSD.ORG> wrote:
> What does the -audit readers think of the following patch?

Fun!  I see the sysctls that don't have a description are actively
being attacked already :')

>  static u_long wantfreevnodes = 25;
> -SYSCTL_LONG(_vfs, OID_AUTO, wantfreevnodes, CTLFLAG_RW,
> &wantfreevnodes, 0, "");
> +SYSCTL_LONG(_vfs, OID_AUTO, wantfreevnodes, CTLFLAG_RW,
> &wantfreevnodes, 0,
> +    "Minimum count of free vnodes");

X-Mailer: Sylpheed version 0.8.10claws (GTK+ 1.2.10; i386-portbld-freebsd5.0)

Evil(TM) mailer-wrapping here, right?  Watch out for those GUI mailers
that have the nasty habit of finding amusement in the destruction of
otherwise fine work :)

> @@ -205,11 +208,14 @@
>  static int syncer_maxdelay = SYNCER_MAXDELAY;	/* maximum delay time */
>  static int syncdelay = 30;		/* max time to delay syncing data */
>  static int filedelay = 30;		/* time to delay syncing files */
> -SYSCTL_INT(_kern, OID_AUTO, filedelay, CTLFLAG_RW, &filedelay, 0, "");
> +SYSCTL_INT(_kern, OID_AUTO, filedelay, CTLFLAG_RW, &filedelay, 0,
> +    "File synchornization delay in seconds");

Typo -----------> ^^ s/synchorn/synchron/

>  static int max_proc_mmap;
> -SYSCTL_INT(_vm, OID_AUTO, max_proc_mmap, CTLFLAG_RW, &max_proc_mmap, 0,
> "");
> +SYSCTL_INT(_vm, OID_AUTO, max_proc_mmap, CTLFLAG_RW, &max_proc_mmap, 0,
> +    "Maximum number of mmap()'ed spaces");

"Maximum number of mmap'ed areas per process" ?

> Index: vm/vm_zeroidle.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/vm/vm_zeroidle.c,v
> retrieving revision 1.18
> diff -u -r1.18 vm_zeroidle.c
> --- vm/vm_zeroidle.c	12 Oct 2002 05:32:24 -0000	1.18
> +++ vm/vm_zeroidle.c	17 Mar 2003 11:22:31 -0000
> @@ -32,11 +32,13 @@
>  	cnt_prezero, CTLFLAG_RD, &cnt_prezero, 0, "");
>
>  static int idlezero_enable = 1;
> -SYSCTL_INT(_vm, OID_AUTO, idlezero_enable, CTLFLAG_RW,
> &idlezero_enable, 0, "");
> +SYSCTL_INT(_vm, OID_AUTO, idlezero_enable, CTLFLAG_RW,
> &idlezero_enable, 0,
> +    "Enable/disable idle zeroing of pages");

"free pages".

If I'm reading the code correctly, this enables the pagezero kernel
task that runs asynchronously in the background with RTP_PRIO_IDLE
priority.  When the system is idle, the pagezero task takes over and
zeroes out pages from the free list stopping its run after zeroing at
most idlezero_maxrun or when a process becomes runnable.

"Enable zeroing out of free virtual memory pages when idle." perhaps?

>  TUNABLE_INT("vm.idlezero_enable", &idlezero_enable);
>
>  static int idlezero_maxrun = 16;
> -SYSCTL_INT(_vm, OID_AUTO, idlezero_maxrun, CTLFLAG_RW,
> &idlezero_maxrun, 0, "");
> +SYSCTL_INT(_vm, OID_AUTO, idlezero_maxrun, CTLFLAG_RW,
> &idlezero_maxrun, 0,
> +    "Maximum iterations for idle zeroing of pages");

"Maximum number of pages to zero out in one run of (pagezero)."  ??

> -SYSCTL_LONG(_vfs, OID_AUTO, wantfreevnodes, CTLFLAG_RW, &wantfreevnodes, 0, "");
> +SYSCTL_LONG(_vfs, OID_AUTO, wantfreevnodes, CTLFLAG_RW, &wantfreevnodes, 0,
> +    "Minimum count of free vnodes");
>  /* Number of vnodes in the free list. */
>  static u_long freevnodes;
> -SYSCTL_LONG(_vfs, OID_AUTO, freevnodes, CTLFLAG_RD, &freevnodes, 0, "");
> +SYSCTL_LONG(_vfs, OID_AUTO, freevnodes, CTLFLAG_RD, &freevnodes, 0,
> +    "Number of vnodes in the free list");

These seem to state the obvious, which is redundant if one already
knows the name of the sysctl :-/

>  #ifdef DIAGNOSTIC
>  static int busyprt = 0;		/* print out busy vnodes */
> -SYSCTL_INT(_debug, OID_AUTO, busyprt, CTLFLAG_RW, &busyprt, 0, "");
> +SYSCTL_INT(_debug, OID_AUTO, busyprt, CTLFLAG_RW, &busyprt, 0,
> +    "Print busy vnodes");

When?  I mean when will the busy vnodes be printed?

There is a lot of space until the description reaches 80 cols.
If we're to add a description here, any details we can add are good.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030318120206.GD25064>