Date: Mon, 6 Nov 2017 00:56:08 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Edward Tomasz Napierala <trasz@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r324857 - in head: lib/libc/gen sys/vm Message-ID: <20171105233555.H976@besplex.bde.org> In-Reply-To: <201710221035.v9MAZUki094587@repo.freebsd.org> References: <201710221035.v9MAZUki094587@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 22 Oct 2017, Edward Tomasz Napierala wrote: > Log: > Add OID for the vm.overcommit sysctl. This makes it possible to remove > one call to sysctl(2) from jemalloc startup code. (That also requires > changes to jemalloc, but I plan to push those to upstream first.) This change has many style bugs, and the ABI regression is unfortunate. Is it really useful to avoid 1 syscall, even in critical code? I prefer to avoid jemalloc (and also malloc) in small utilities. In non-small utilities, 1 more syscall doesn't matter. > Modified: head/lib/libc/gen/sysctl.3 > ============================================================================== > --- head/lib/libc/gen/sysctl.3 Sun Oct 22 10:32:40 2017 (r324856) > +++ head/lib/libc/gen/sysctl.3 Sun Oct 22 10:35:29 2017 (r324857) > @@ -28,7 +28,7 @@ > .\" @(#)sysctl.3 8.4 (Berkeley) 5/9/95 > .\" $FreeBSD$ > .\" > -.Dd September 10, 2015 > +.Dd October 22, 2017 > .Dt SYSCTL 3 > .Os > .Sh NAME > @@ -741,6 +741,7 @@ privilege may change the value. > .It Dv VM_V_FREE_TARGET Ta integer Ta yes > .It Dv VM_V_INACTIVE_TARGET Ta integer Ta yes > .It Dv VM_V_PAGEOUT_FREE_MIN Ta integer Ta yes > +.It Dv VM_OVERCOMMIT Ta integer Ta yes > .El > .Bl -tag -width 6n > .It Li VM_LOADAVG This unsorts the list. Most lists in sysctl(3) were almost sorted, and this one was unsorted. The main unsortings and nearby errors now are: - CTL_VFS is unsorted in the top-level list (and description) - CTL_P1003_1B is missing in the top-level list (and description) - the second level for CTL_P1003_1B has more compile-time-constant numeric ids than most; these are also missing - CTL_*MAXID values were bogus/unused. All of them except the one for CTL_P1003_1B have been removed in sys/sysctl.h. - the second level for CTL_HW is in numeric order which is random/historical alphabetically - the second level KERN_HOSTNAME is unsorted - the second level KERN_ARND, KERN_DUMPDEV, KERN_IOV_MAX, KERN_IPC, KERN_LOGSIGEXIT, KERN_MAXPHYS, KERN_NTP_PLL, KERN_PS_STRINGS and KERN_USRSTACK are missing. All of these are relatively new and probably shouldn't exist as compile-time constants. - the second level KERN_QUANTUM is not missing, but doesn't exist. Its numeric value was renamed to KERN_DUMMY, and its name was changed to kern.sched.quantum. It should never have existed as a compile-time constant, but it should be documented. Its new name is of course not documented in any many page. Nothing in kern.sched is not documented in any many page. sysctl -d kern.sched gives some documentation. - the second level of KERN_PROF is in numeric/historical order - the second level PF_ROUTE is unsorted - the fifth level NET_RT_FLAGS is unsorted and not described - the fifth level NET_RT_IFLISTL is unsorted - sysctls without a compile-time-constant numeric id are mostly missing in sysctl(3). The second and lower levels of FP_INET are exceptions which show how things should be done. > @@ -773,6 +774,9 @@ process address space when needed. > .It Li VM_V_PAGEOUT_FREE_MIN > If the amount of free and cache memory falls below this value, the > pageout daemon will enter "memory conserving mode" to avoid deadlock. > +.It Li VM_OVERCOMMIT > +Overcommit behaviour, as described in > +.Xr tuning 7 . This is consistently unsorted. U.S. spelling for behavior, etc., should be used in most places in FreeBSD (except where the nearby spelling is already British). Many references to tuning(7) are bogus, but this one is correct, except I think the reference should be from tuning(7) to here instead of vice versa. > .El > .Sh RETURN VALUES > .Rv -std > > Modified: head/sys/vm/swap_pager.c > ============================================================================== > --- head/sys/vm/swap_pager.c Sun Oct 22 10:32:40 2017 (r324856) > +++ head/sys/vm/swap_pager.c Sun Oct 22 10:35:29 2017 (r324857) > @@ -157,7 +157,7 @@ static vm_ooffset_t swap_reserved; > SYSCTL_QUAD(_vm, OID_AUTO, swap_reserved, CTLFLAG_RD, &swap_reserved, 0, > "Amount of swap storage needed to back all allocated anonymous memory."); > static int overcommit = 0; > -SYSCTL_INT(_vm, OID_AUTO, overcommit, CTLFLAG_RW, &overcommit, 0, > +SYSCTL_INT(_vm, VM_OVERCOMMIT, overcommit, CTLFLAG_RW, &overcommit, 0, > "Configure virtual memory overcommit behavior. See tuning(7) " > "for details."); This reference to tuning(7) is bogus. The string has several style bugs. It is verbose, and the reference to the man page is mostly padding. Every sysctl should be described in more detail in a man page. Preferably only one big one, or one whose name is consistently related to the sysctl name, so that it is easy to find. > static unsigned long swzone; > > Modified: head/sys/vm/vm_param.h > ============================================================================== > --- head/sys/vm/vm_param.h Sun Oct 22 10:32:40 2017 (r324856) > +++ head/sys/vm/vm_param.h Sun Oct 22 10:35:29 2017 (r324857) > @@ -84,7 +84,8 @@ > #define VM_V_PAGEOUT_FREE_MIN 9 /* vm_cnt.v_pageout_free_min */ > #define VM_OBSOLETE_10 10 /* pageout algorithm */ > #define VM_SWAPPING_ENABLED 11 /* swapping enabled */ > -#define VM_MAXID 12 /* number of valid vm ids */ > +#define VM_OVERCOMMIT 12 /* vm.overcommit */ > +#define VM_MAXID 13 /* number of valid vm ids */ VM_OVERCOMMIT is misindented like 2 of the 4 nearby visible definitions. VM_OVERCOMMIT has a banal comment, like 1 nearby visible definitions. MAXID definitions haven't been removed for lower levels yet, but if VM_MAXID were actually used then incrementing it would give ABI problems. > > /* > * Structure for swap device statistics Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171105233555.H976>