Project

General

Profile

Bug #10291

topo_dprintf should evaluate debug mask before forging ahead

Added by Rob Johnston 11 months ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-01-25
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

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.

Note - this work has already been integrated into illumos-joyent via the commit below:

commit 76683f77aeee2619b447d8ea5843399835fbc586
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   Tue Sep 11 22:37:24 2018 +0000

    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>

So this issue is really to track getting the above commit upstreamed to illumos-gate.

For details on how the change was tested, refer to the SmartOS bug report:

https://smartos.org/bugview/OS-7230

History

#1

Updated by Electric Monk 10 months ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

git commit 1de1e652632a9912511ab1cd8c8c4628d5e5f1da

commit  1de1e652632a9912511ab1cd8c8c4628d5e5f1da
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   2019-01-28T22:45:31.000Z

    10291 topo_dprintf should evaluate debug mask before forging ahead
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Gergő Mihály Doma <domag02@gmail.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF