OS-7230: topo_dprintf should evaluate debug mask before forging ahead


Issue Type:Bug
Priority:4 - Normal
Created at:2018-09-07T20:39:14.637Z
Updated at:2018-09-12T23:02:09.963Z


Created by:Rob Johnston
Reported by:Rob Johnston
Assigned to:Rob Johnston


Fixed: A fix for this issue is checked into the tree and tested.
(Resolution Date: 2018-09-12T23:02:09.951Z)

Fix Versions

2018-09-13 Astronaut Mike Dexter (Release Date: 2018-09-13)

Related Links


topo_dprintf() is used by the core libtopo code to conditionally emit debug messages based on a debug mask setting.

The implementation looks like this:

topo_dprintf(topo_hdl_t *thp, int mask, const char *format, ...)
	va_list ap;

	va_start(ap, format);
	topo_vdprintf(thp, mask, NULL, format, ap);

And the actual comparison of the configured debug level against the debug mask doesn't occur till it calls topo_vdprintf(). As a result, we take the performace hit of doing the varargs processing every single time regardless of whether it later decides to suppress the message. It really should do the debug mask comparison as a first step. - which is what topo_mod_dprintf() does.


Comment by Rob Johnston
Created at 2018-09-10T23:16:02.433Z


Built a PI with the change. Then ran fmtopo with various combinations of TOPO_DEBUG env var settings and verified that the expected debug output was emitted when enabled.

Comment by Jira Bot
Created at 2018-09-12T22:58:00.029Z

illumos-joyent commit 76683f77aeee2619b447d8ea5843399835fbc586 (branch master, by Rob Johnston)

OS-7230 topo_dprintf should evaluate debug mask before forging ahead
OS-7228 Add percentage unit type to sensor abstraction layer
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: Patrick Mooney <patrick.mooney@joyent.com>