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
This commit is contained in:
Conrad Meyer 2019-05-07 17:47:20 +00:00
parent 5350e15d0d
commit 7d7db5298d
9 changed files with 224 additions and 16 deletions

View File

@ -37,5 +37,6 @@ FBSD_1.4 {
FBSD_1.5 {
sbuf_putbuf;
sbuf_printf_drain;
};

View File

@ -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

View File

@ -59,6 +59,60 @@ sbuf_vprintf_helper(struct sbuf *sb, const char * restrict format, ...)
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);

View File

@ -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 \

View File

@ -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 drain operations to avoid
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 buffer has an error.
.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

View File

@ -322,13 +322,6 @@ sysctl_load_tunable_by_oid_locked(struct sysctl_oid *oidp)
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

View File

@ -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 = 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));
retval = device_print_prettyname(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);
}

View File

@ -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);
}

View File

@ -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