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

Details

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

People

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

Resolution

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

Description

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:

void
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);
	va_end(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.

Comments

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

Testing

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>