On Tue, Mar 25, 2014 at 11:56:24AM +0200, Jani Nikula wrote:
On Mon, 24 Mar 2014, Damien Lespiau damien.lespiau@intel.com wrote:
In the logging code, we are currently checking is we need to output in
s/is/if/
drm_ut_debug_printk(). This is too late. The problem is that when we write something like:
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", connector->base.id, drm_get_connector_name(connector), connector->encoder->base.id, drm_get_encoder_name(connector->encoder));We start by evaluating the arguments (so call drm_get_connector_name() and drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will then does nothing.
s/does/do/
This means we execute a lot of instructions (drm_get_connector_name(), in turn, calls snprintf() for example) to happily discard them in the normal case, drm.debug=0.
So, let's put the test on drm_debug earlier, in the macros themselves. Sprinkle an unlikely() as well for good measure.
Bikeshed, is it likely that the unlikely matters all that much? https://lwn.net/Articles/70473/
I won't insist on removing them, it's more the casual nature of the "sprinkling unlikely for good measure" that bugs me.
I've considered this and since most users don't set debug every we should be easily able to hit the 1:1M ratio seemingly required to make debug work.
And once you enable debugging the actual printk overhead will wash out anything you pay in branch mispredicts anyway. -Daniel
BR, Jani.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com
drivers/gpu/drm/drm_stub.c | 26 ++++++++++++-------------- include/drm/drmP.h | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index dc2c609..81ed832 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...) } EXPORT_SYMBOL(drm_err);
-void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...) { struct va_format vaf; va_list args;
- if (drm_debug & request_level) {
va_start(args, format);vaf.fmt = format;vaf.va = &args;if (function_name)printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);elseprintk(KERN_DEBUG "%pV", &vaf);va_end(args);- }
- va_start(args, format);
- vaf.fmt = format;
- vaf.va = &args;
- if (function_name)
printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);- else
printk(KERN_DEBUG "%pV", &vaf);- va_end(args);
} EXPORT_SYMBOL(drm_ut_debug_printk);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3055b36..87b558a 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -121,9 +121,8 @@ struct videomode; #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08
-extern __printf(4, 5) -void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+extern __printf(3, 4) +void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...); extern __printf(2, 3) @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...); #if DRM_DEBUG_CODE #define DRM_DEBUG(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_CORE)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_DRIVER(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_DRIVER)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \-#define DRM_DEBUG_KMS(fmt, args...) \ +#define DRM_DEBUG_KMS(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_KMS)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_PRIME(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_PRIME)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#else
#define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel