svn commit: r347229 - in head: lib/libsbuf lib/libsbuf/tests share/man/man9 sys/kern sys/sys
Rodney W. Grimes
freebsd at gndrsh.dnsmgr.net
Tue May 7 17:52:24 UTC 2019
> Author: cem
> Date: Tue May 7 17:47:20 2019
> New Revision: 347229
> URL: https://svnweb.freebsd.org/changeset/base/347229
>
> Log:
> device_printf: Use sbuf for more coherent prints on SMP
>
> device_printf does multiple calls to printf allowing other console messages to
> be inserted between the device name, and the rest of the message. This change
> uses sbuf to compose to two into a single buffer, and prints it all at once.
>
> It exposes an sbuf drain function (drain-to-printf) for common use.
>
> Update documentation to match; some unit tests included.
>
> Submitted by: jmg
> Sponsored by: Dell EMC Isilon
> Differential Revision: https://reviews.freebsd.org/D16690
Thank you! this has been annoying me for a while,
I knew it was going on, but wasnt sure where it was coming from.
Does this code MFC back to 12 and 11 easily?
> Modified:
> head/lib/libsbuf/Symbol.map
> head/lib/libsbuf/tests/sbuf_core_test.c
> head/lib/libsbuf/tests/sbuf_stdio_test.c
> head/share/man/man9/Makefile
> head/share/man/man9/sbuf.9
> head/sys/kern/kern_sysctl.c
> head/sys/kern/subr_bus.c
> head/sys/kern/subr_prf.c
> head/sys/sys/sbuf.h
>
> Modified: head/lib/libsbuf/Symbol.map
> ==============================================================================
> --- head/lib/libsbuf/Symbol.map Tue May 7 16:17:33 2019 (r347228)
> +++ head/lib/libsbuf/Symbol.map Tue May 7 17:47:20 2019 (r347229)
> @@ -37,5 +37,6 @@ FBSD_1.4 {
>
> FBSD_1.5 {
> sbuf_putbuf;
> + sbuf_printf_drain;
> };
>
>
> Modified: head/lib/libsbuf/tests/sbuf_core_test.c
> ==============================================================================
> --- head/lib/libsbuf/tests/sbuf_core_test.c Tue May 7 16:17:33 2019 (r347228)
> +++ head/lib/libsbuf/tests/sbuf_core_test.c Tue May 7 17:47:20 2019 (r347229)
> @@ -63,6 +63,9 @@ ATF_TC_BODY(sbuf_clear_test, tc)
> */
> child_proc = atf_utils_fork();
> if (child_proc == 0) {
> + ATF_REQUIRE_EQ_MSG(0, sbuf_finish(sb), "sbuf_finish failed: %s",
> + strerror(errno));
> +
> sbuf_putbuf(sb);
> exit(0);
> }
> @@ -100,6 +103,34 @@ ATF_TC_BODY(sbuf_done_and_sbuf_finish_test, tc)
> sbuf_delete(sb);
> }
>
> +static int
> +drain_ret0(void *arg, const char *data, int len)
> +{
> +
> + (void)arg;
> + (void)data;
> + (void)len;
> +
> + return (0);
> +}
> +
> +ATF_TC_WITHOUT_HEAD(sbuf_drain_ret0_test);
> +ATF_TC_BODY(sbuf_drain_ret0_test, tc)
> +{
> + struct sbuf *sb;
> +
> + sb = sbuf_new_auto();
> +
> + sbuf_set_drain(sb, drain_ret0, NULL);
> +
> + sbuf_cat(sb, test_string);
> +
> + ATF_CHECK_EQ_MSG(-1, sbuf_finish(sb),
> + "required to return error when drain func returns 0");
> + ATF_CHECK_EQ_MSG(EDEADLK, errno,
> + "errno required to be EDEADLK when drain func returns 0");
> +}
> +
> ATF_TC_WITHOUT_HEAD(sbuf_len_test);
> ATF_TC_BODY(sbuf_len_test, tc)
> {
> @@ -131,6 +162,34 @@ ATF_TC_BODY(sbuf_len_test, tc)
> sbuf_delete(sb);
> }
>
> +ATF_TC_WITHOUT_HEAD(sbuf_new_fixedlen);
> +ATF_TC_BODY(sbuf_new_fixedlen, tc)
> +{
> + char buf[strlen(test_string) + 1];
> + struct sbuf sb;
> + pid_t child_proc;
> +
> + sbuf_new(&sb, buf, sizeof(buf), SBUF_FIXEDLEN);
> +
> + sbuf_cat(&sb, test_string);
> +
> + child_proc = atf_utils_fork();
> + if (child_proc == 0) {
> + ATF_REQUIRE_EQ_MSG(0, sbuf_finish(&sb), "sbuf_finish failed: %s",
> + strerror(errno));
> +
> + sbuf_putbuf(&sb);
> + exit(0);
> + }
> + atf_utils_wait(child_proc, 0, test_string, "");
> +
> + sbuf_putc(&sb, ' ');
> +
> + ATF_CHECK_EQ_MSG(-1, sbuf_finish(&sb), "failed to return error on overflow");
> +
> + sbuf_delete(&sb);
> +}
> +
> ATF_TC_WITHOUT_HEAD(sbuf_setpos_test);
> ATF_TC_BODY(sbuf_setpos_test, tc)
> {
> @@ -190,7 +249,9 @@ ATF_TP_ADD_TCS(tp)
>
> ATF_TP_ADD_TC(tp, sbuf_clear_test);
> ATF_TP_ADD_TC(tp, sbuf_done_and_sbuf_finish_test);
> + ATF_TP_ADD_TC(tp, sbuf_drain_ret0_test);
> ATF_TP_ADD_TC(tp, sbuf_len_test);
> + ATF_TP_ADD_TC(tp, sbuf_new_fixedlen);
> #if 0
> /* TODO */
> #ifdef HAVE_SBUF_CLEAR_FLAGS
>
> Modified: head/lib/libsbuf/tests/sbuf_stdio_test.c
> ==============================================================================
> --- head/lib/libsbuf/tests/sbuf_stdio_test.c Tue May 7 16:17:33 2019 (r347228)
> +++ head/lib/libsbuf/tests/sbuf_stdio_test.c Tue May 7 17:47:20 2019 (r347229)
> @@ -59,6 +59,60 @@ sbuf_vprintf_helper(struct sbuf *sb, const char * rest
> return (rc);
> }
>
> +ATF_TC_WITHOUT_HEAD(sbuf_printf_drain_null_test);
> +ATF_TC_BODY(sbuf_printf_drain_null_test, tc)
> +{
> + struct sbuf *sb;
> + char buf[2];
> + pid_t child_proc;
> +
> + sb = sbuf_new(NULL, buf, sizeof(buf), SBUF_FIXEDLEN);
> + ATF_REQUIRE_MSG(sb != NULL, "sbuf_new_auto failed: %s",
> + strerror(errno));
> +
> + child_proc = atf_utils_fork();
> + if (child_proc == 0) {
> + sbuf_set_drain(sb, sbuf_printf_drain, NULL);
> +
> + ATF_REQUIRE_EQ_MSG(0, sbuf_cat(sb, test_string),
> + "sbuf_cat failed");
> +
> + ATF_CHECK_EQ(0, sbuf_finish(sb));
> + exit(0);
> + }
> + atf_utils_wait(child_proc, 0, test_string, "");
> +
> + sbuf_delete(sb);
> +}
> +
> +ATF_TC_WITHOUT_HEAD(sbuf_printf_drain_test);
> +ATF_TC_BODY(sbuf_printf_drain_test, tc)
> +{
> + struct sbuf *sb;
> + char buf[2];
> + pid_t child_proc;
> + size_t cnt = 0;
> +
> + sb = sbuf_new(NULL, buf, sizeof(buf), SBUF_FIXEDLEN);
> + ATF_REQUIRE_MSG(sb != NULL, "sbuf_new_auto failed: %s",
> + strerror(errno));
> +
> + child_proc = atf_utils_fork();
> + if (child_proc == 0) {
> + sbuf_set_drain(sb, sbuf_printf_drain, &cnt);
> +
> + ATF_REQUIRE_EQ_MSG(0, sbuf_cat(sb, test_string),
> + "sbuf_cat failed");
> +
> + ATF_CHECK_EQ(0, sbuf_finish(sb));
> + ATF_CHECK_EQ(strlen(test_string), cnt);
> + exit(0);
> + }
> + atf_utils_wait(child_proc, 0, test_string, "");
> +
> + sbuf_delete(sb);
> +}
> +
> ATF_TC_WITHOUT_HEAD(sbuf_printf_test);
> ATF_TC_BODY(sbuf_printf_test, tc)
> {
> @@ -106,6 +160,7 @@ ATF_TC_BODY(sbuf_putbuf_test, tc)
>
> child_proc = atf_utils_fork();
> if (child_proc == 0) {
> + ATF_CHECK_EQ(0, sbuf_finish(sb));
> sbuf_putbuf(sb);
> exit(0);
> }
> @@ -152,6 +207,8 @@ ATF_TC_BODY(sbuf_vprintf_test, tc)
> ATF_TP_ADD_TCS(tp)
> {
>
> + ATF_TP_ADD_TC(tp, sbuf_printf_drain_null_test);
> + ATF_TP_ADD_TC(tp, sbuf_printf_drain_test);
> ATF_TP_ADD_TC(tp, sbuf_printf_test);
> ATF_TP_ADD_TC(tp, sbuf_putbuf_test);
> ATF_TP_ADD_TC(tp, sbuf_vprintf_test);
>
> Modified: head/share/man/man9/Makefile
> ==============================================================================
> --- head/share/man/man9/Makefile Tue May 7 16:17:33 2019 (r347228)
> +++ head/share/man/man9/Makefile Tue May 7 17:47:20 2019 (r347229)
> @@ -1780,6 +1780,8 @@ MLINKS+=sbuf.9 sbuf_bcat.9 \
> sbuf.9 sbuf_new_auto.9 \
> sbuf.9 sbuf_new_for_sysctl.9 \
> sbuf.9 sbuf_printf.9 \
> + sbuf.9 sbuf_printf_drain.9 \
> + sbuf.9 sbuf_putbuf.9 \
> sbuf.9 sbuf_putc.9 \
> sbuf.9 sbuf_set_drain.9 \
> sbuf.9 sbuf_set_flags.9 \
>
> Modified: head/share/man/man9/sbuf.9
> ==============================================================================
> --- head/share/man/man9/sbuf.9 Tue May 7 16:17:33 2019 (r347228)
> +++ head/share/man/man9/sbuf.9 Tue May 7 17:47:20 2019 (r347229)
> @@ -25,7 +25,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd May 23, 2018
> +.Dd May 7, 2019
> .Dt SBUF 9
> .Os
> .Sh NAME
> @@ -58,6 +58,7 @@
> .Nm sbuf_start_section ,
> .Nm sbuf_end_section ,
> .Nm sbuf_hexdump ,
> +.Nm sbuf_printf_drain ,
> .Nm sbuf_putbuf
> .Nd safe string composition
> .Sh SYNOPSIS
> @@ -191,6 +192,12 @@
> .Fa "const char *hdr"
> .Fa "int flags"
> .Fc
> +.Ft int
> +.Fo sbuf_printf_drain
> +.Fa "void *arg"
> +.Fa "const char *data"
> +.Fa "int len"
> +.Fc
> .Ft void
> .Fo sbuf_putbuf
> .Fa "struct sbuf *s"
> @@ -278,7 +285,9 @@ as a record boundary marker that will be used during d
> records being split.
> If a record grows sufficiently large such that it fills the
> .Fa sbuf
> -and therefore cannot be drained without being split, an error of EDEADLK is set.
> +and therefore cannot be drained without being split, an error of
> +.Er EDEADLK
> +is set.
> .El
> .Pp
> Note that if
> @@ -419,7 +428,9 @@ many bytes were drained.
> If negative, the return value indicates the negative error code which
> will be returned from this or a later call to
> .Fn sbuf_finish .
> -The returned drained length cannot be zero.
> +If the returned drained length is 0, an error of
> +.Er EDEADLK
> +is set.
> To do unbuffered draining, initialize the sbuf with a two-byte buffer.
> The drain will be called for every byte added to the sbuf.
> The
> @@ -492,7 +503,9 @@ The
> .Fn sbuf_error
> function returns any error value that the
> .Fa sbuf
> -may have accumulated, either from the drain function, or ENOMEM if the
> +may have accumulated, either from the drain function, or
> +.Er ENOMEM
> +if the
> .Fa sbuf
> overflowed.
> This function is generally not needed and instead the error code from
> @@ -574,9 +587,29 @@ See the
> man page for more details on the interface.
> .Pp
> The
> +.Fn sbuf_printf_drain
> +function is a drain function that will call printf, or log to the console.
> +The argument
> +.Fa arg
> +must be either
> +.Dv NULL ,
> +or a valid pointer to a
> +.Vt size_t .
> +If
> +.Fa arg
> +is not
> +.Dv NULL ,
> +the total bytes drained will be added to the value pointed to by
> +.Fa arg .
> +.Pp
> +The
> .Fn sbuf_putbuf
> function printfs the sbuf to stdout if in userland, and to the console
> and log if in the kernel.
> +The
> +.Fa sbuf
> +must be finished before calling
> +.Fn sbuf_putbuf .
> It does not drain the buffer or update any pointers.
> .Sh NOTES
> If an operation caused an
> @@ -655,8 +688,9 @@ function returns the section length or \-1 if the buff
> .Pp
> The
> .Fn sbuf_finish 9
> -function (the kernel version) returns ENOMEM if the sbuf overflowed before
> -being finished,
> +function (the kernel version) returns
> +.Er ENOMEM
> +if the sbuf overflowed before being finished,
> or returns the error code from the drain if one is attached.
> .Pp
> The
>
> Modified: head/sys/kern/kern_sysctl.c
> ==============================================================================
> --- head/sys/kern/kern_sysctl.c Tue May 7 16:17:33 2019 (r347228)
> +++ head/sys/kern/kern_sysctl.c Tue May 7 17:47:20 2019 (r347229)
> @@ -322,13 +322,6 @@ sysctl_load_tunable_by_oid_locked(struct sysctl_oid *o
> freeenv(penv);
> }
>
> -static int
> -sbuf_printf_drain(void *arg __unused, const char *data, int len)
> -{
> -
> - return (printf("%.*s", len, data));
> -}
> -
> /*
> * Locate the path to a given oid. Returns the length of the resulting path,
> * or -1 if the oid was not found. nodes must have room for CTL_MAXNAME
>
> Modified: head/sys/kern/subr_bus.c
> ==============================================================================
> --- head/sys/kern/subr_bus.c Tue May 7 16:17:33 2019 (r347228)
> +++ head/sys/kern/subr_bus.c Tue May 7 17:47:20 2019 (r347229)
> @@ -2431,13 +2431,31 @@ device_print_prettyname(device_t dev)
> int
> device_printf(device_t dev, const char * fmt, ...)
> {
> + char buf[128];
> + struct sbuf sb;
> + const char *name;
> va_list ap;
> - int retval;
> + size_t retval;
>
> - retval = device_print_prettyname(dev);
> + retval = 0;
> +
> + sbuf_new(&sb, buf, sizeof(buf), SBUF_FIXEDLEN);
> + sbuf_set_drain(&sb, sbuf_printf_drain, &retval);
> +
> + name = device_get_name(dev);
> +
> + if (name == NULL)
> + sbuf_cat(&sb, "unknown: ");
> + else
> + sbuf_printf(&sb, "%s%d: ", name, device_get_unit(dev));
> +
> va_start(ap, fmt);
> - retval += vprintf(fmt, ap);
> + sbuf_vprintf(&sb, fmt, ap);
> va_end(ap);
> +
> + sbuf_finish(&sb);
> + sbuf_delete(&sb);
> +
> return (retval);
> }
>
>
> Modified: head/sys/kern/subr_prf.c
> ==============================================================================
> --- head/sys/kern/subr_prf.c Tue May 7 16:17:33 2019 (r347228)
> +++ head/sys/kern/subr_prf.c Tue May 7 17:47:20 2019 (r347229)
> @@ -62,6 +62,8 @@ __FBSDID("$FreeBSD$");
> #include <sys/syslog.h>
> #include <sys/cons.h>
> #include <sys/uio.h>
> +#else /* !_KERNEL */
> +#include <errno.h>
> #endif
> #include <sys/ctype.h>
> #include <sys/sbuf.h>
> @@ -1254,3 +1256,42 @@ sbuf_putbuf(struct sbuf *sb)
> printf("%s", sbuf_data(sb));
> }
> #endif
> +
> +int
> +sbuf_printf_drain(void *arg, const char *data, int len)
> +{
> + size_t *retvalptr;
> + int r;
> +#ifdef _KERNEL
> + char *dataptr;
> + char oldchr;
> +
> + /*
> + * This is allowed as an extra byte is always resvered for
> + * terminating NUL byte. Save and restore the byte because
> + * we might be flushing a record, and there may be valid
> + * data after the buffer.
> + */
> + oldchr = data[len];
> + dataptr = __DECONST(char *, data);
> + dataptr[len] = '\0';
> +
> + prf_putbuf(dataptr, TOLOG | TOCONS, -1);
> + r = len;
> +
> + dataptr[len] = oldchr;
> +
> +#else /* !_KERNEL */
> +
> + r = printf("%.*s", len, data);
> + if (r < 0)
> + return (-errno);
> +
> +#endif
> +
> + retvalptr = arg;
> + if (retvalptr != NULL)
> + *retvalptr += r;
> +
> + return (r);
> +}
>
> Modified: head/sys/sys/sbuf.h
> ==============================================================================
> --- head/sys/sys/sbuf.h Tue May 7 16:17:33 2019 (r347228)
> +++ head/sys/sys/sbuf.h Tue May 7 17:47:20 2019 (r347229)
> @@ -103,6 +103,7 @@ void sbuf_start_section(struct sbuf *, ssize_t *);
> ssize_t sbuf_end_section(struct sbuf *, ssize_t, size_t, int);
> void sbuf_hexdump(struct sbuf *, const void *, int, const char *,
> int);
> +int sbuf_printf_drain(void *arg, const char *data, int len);
> void sbuf_putbuf(struct sbuf *);
>
> #ifdef _KERNEL
>
>
--
Rod Grimes rgrimes at freebsd.org
More information about the svn-src-all
mailing list