Re: [PATCH 09/11 re-post] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions

Joe Perches
2011-08-12T06:39:27Z

On Thu, 2011-08-11 at 16:52 -0400, Jason Baron wrote:
> Replace the repetitive struct _ddebug descriptor definitions with
> a new DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) macro.
[]
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
[]
> @@ -54,33 +54,28 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
> const char *fmt, ...)
> __attribute__ ((format (printf, 3, 4)));
>
> +#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
> + static struct _ddebug name \
> + __used \
> + __attribute__((section("__verbose"), aligned(8))) = \
> + { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
> + _DPRINTK_FLAGS_DEFAULT }

Hey again Jason.

>From a kernel coding style consistency POV, shouldn't
this use c99 named initializers?

#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
static struct _ddebug name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
.format = fmt, \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
.enabled = false, \
} __used __aligned(8) __attribute__((section("__verbose")))

If the pr_debug/dynamic_debug inversion is done,
the .enabled flag should be set on as well when
-DDEBUG is set, correct?

> #define dynamic_pr_debug(fmt, ...) do { \
> - static struct _ddebug descriptor \
> - __used \
> - __attribute__((section("__verbose"), aligned(8))) = \
> - { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
> - _DPRINTK_FLAGS_DEFAULT }; \
> + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> if (unlikely(descriptor.enabled)) \
> - __dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__); \
> + __dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__);\
> } while (0)

Perhaps

#define dynamic_pr_debug(fmt, ...) \
do { \
DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.enabled)) \
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
##__VA_ARGS__); \
} while (0)

Lastly, are the unlikely()s really useful?