svn commit: r353283 - in head: lib lib/libstats share/man/man3 share/mk sys/amd64/conf sys/conf sys/kern sys/sys tools/build/options

Kyle Evans kevans at freebsd.org
Tue Jul 14 13:38:52 UTC 2020


On Mon, Oct 7, 2019 at 2:05 PM Edward Tomasz Napierala
<trasz at freebsd.org> wrote:
>
> Author: trasz
> Date: Mon Oct  7 19:05:05 2019
> New Revision: 353283
> URL: https://svnweb.freebsd.org/changeset/base/353283
>
> Log:
>   Introduce stats(3), a flexible statistics gathering API.
>
>   This provides a framework to define a template describing
>   a set of "variables of interest" and the intended way for
>   the framework to maintain them (for example the maximum, sum,
>   t-digest, or a combination thereof).  Afterwards the user
>   code feeds in the raw data, and the framework maintains
>   these variables inside a user-provided, opaque stats blobs.
>   The framework also provides a way to selectively extract the
>   stats from the blobs.  The stats(3) framework can be used in
>   both userspace and the kernel.
>
>   See the stats(3) manual page for details.
>
>   This will be used by the upcoming TCP statistics gathering code,
>   https://reviews.freebsd.org/D20655.
>
>   The stats(3) framework is disabled by default for now, except
>   in the NOTES kernel (for QA); it is expected to be enabled
>   in amd64 GENERIC after a cool down period.
>
>   Reviewed by:  sef (earlier version)
>   Obtained from:        Netflix
>   Relnotes:     yes
>   Sponsored by: Klara Inc, Netflix
>   Differential Revision:        https://reviews.freebsd.org/D20477
>
> Added:
>   head/lib/libstats/
>   head/lib/libstats/Makefile   (contents, props changed)
>   head/share/man/man3/stats.3   (contents, props changed)
>   head/sys/kern/subr_stats.c   (contents, props changed)
>   head/sys/sys/stats.h   (contents, props changed)
>   head/tools/build/options/WITHOUT_STATS   (contents, props changed)
>   head/tools/build/options/WITH_STATS   (contents, props changed)
> Modified:
>   head/lib/Makefile
>   head/share/man/man3/Makefile
>   head/share/man/man3/arb.3
>   head/share/mk/bsd.libnames.mk
>   head/share/mk/src.libnames.mk
>   head/share/mk/src.opts.mk
>   head/sys/amd64/conf/NOTES
>   head/sys/conf/files
>   head/sys/conf/options
>   head/sys/sys/arb.h
>
> Modified: head/lib/Makefile
> ==============================================================================
> --- head/lib/Makefile   Mon Oct  7 18:55:40 2019        (r353282)
> +++ head/lib/Makefile   Mon Oct  7 19:05:05 2019        (r353283)
> @@ -152,6 +152,7 @@ SUBDIR.${MK_GSSAPI}+=       libgssapi librpcsec_gss
>  SUBDIR.${MK_ICONV}+=   libiconv_modules
>  SUBDIR.${MK_KERBEROS_SUPPORT}+=        libcom_err
>  SUBDIR.${MK_LDNS}+=    libldns
> +SUBDIR.${MK_STATS}+=   libstats
>
>  # The libraries under libclang_rt can only be built by clang, and only make
>  # sense to build when clang is enabled at all.  Furthermore, they can only be
>
> Added: head/lib/libstats/Makefile
> ==============================================================================
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/lib/libstats/Makefile  Mon Oct  7 19:05:05 2019        (r353283)
> @@ -0,0 +1,14 @@
> +# $FreeBSD$
> +
> +LIB=           stats
> +SHLIBDIR?=     /lib
> +SHLIB_MAJOR=   0
> +SRCS=          subr_stats.c
> +
> +# To debug, comment WITHOUT_ASSERT_DEBUG= and uncomment CFLAGS:=
> +WITHOUT_ASSERT_DEBUG=
> +#CFLAGS:=${CFLAGS:C/-O[0-9]/-O0 -g3/} -DDIAGNOSTIC
> +

Hi,

What exactly is going on here? mjg pointed this out when we were
looking at some runtime assertion related stuff. This looks like it's
imposing an opinion of how it should be built and circumvent the
normal way of doing things. Ideally, this would something more like
with the following patch to just make sure that the CFLAGS
manipulations properly happen when ASSERT_DEBUG is flipped on, and
interested parties that don't want the assertions should turn
ASSERT_DEBUG off. If there's a really really solid reason for libstats
having its own knob, that should be considered as a formal knob rather
than the ad-hockery that appears above. I'm not sure that the
following patch is entirely correct, though; -DDIAGNOSTIC seems to be
needed for assertions, but the -O$n replacement with -O0 -g3 looks
like it should perhaps be split out to a different knob or..
something.

diff --git a/lib/libstats/Makefile b/lib/libstats/Makefile
index 89e10aa64c5..12345d1a61f 100644
--- a/lib/libstats/Makefile
+++ b/lib/libstats/Makefile
@@ -1,13 +1,16 @@
 # $FreeBSD$

+.include <src.opts.mk>
+
 LIB=           stats
 SHLIBDIR?=     /lib
 SHLIB_MAJOR=   0
 SRCS=          subr_stats.c tcp_stats.c

 # To debug, comment WITHOUT_ASSERT_DEBUG= and uncomment CFLAGS:=
-WITHOUT_ASSERT_DEBUG=
-#CFLAGS:=${CFLAGS:C/-O[0-9]/-O0 -g3/} -DDIAGNOSTIC
+.if ${MK_ASSERT_DEBUG} != "no"
+CFLAGS:=${CFLAGS:C/-O[0-9]/-O0 -g3/} -DDIAGNOSTIC
+.endif

 .PATH: ${.CURDIR}/../../sys/kern ${.CURDIR}/../../sys/netinet


More information about the svn-src-all mailing list