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

Jason Baron
2011-08-11T20:53:00Z

On Thu, Aug 11, 2011 at 12:02:06PM -0700, Joe Perches wrote:
> > From: Jason Baron <jbaron@redhat.com>
> > Replace the repetitive struct _ddebug descriptor definitions with
> > a new DYNAMIC_DEBUG_META_DATA(fmt) macro.
>
> Hey Jason.
>
> I think some improvements can be made to this one.
>
> > 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 DYNAMIC_DEBUG_METADATA(fmt) \
> > + static struct _ddebug descriptor \
> > + __used \
> > + __attribute__((section("__verbose"), aligned(8))) = \
> > + { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
> > + _DPRINTK_FLAGS_DEFAULT };
> > +
>
> I think this is unclear and should be:
>
> #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 }
>
> (extra semicolon at end removed)
>
> so the uses become:
>
> > #define dynamic_dev_dbg(dev, fmt, ...) do { \
> > - static struct _ddebug descriptor \
> > - __used \
> > - __attribute__((section("__verbose"), aligned(8))) = \
> > - { KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
> > - _DPRINTK_FLAGS_DEFAULT }; \
> > + DYNAMIC_DEBUG_METADATA(fmt); \
>
> DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);
>
> > if (unlikely(descriptor.enabled)) \
> > - __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__); \
> > + __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
> > } while (0)
>
> so then there aren't any magic variable names.
>
> cheers, Joe
>

Ok, Thanks for the review. Here's a re-post of it:

From: Jason Baron <jbaron@redhat.com>

Replace the repetitive struct _ddebug descriptor definitions with
a new DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) macro.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---

include/linux/dynamic_debug.h | 33 ++++++++++++++-------------------
1 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index feaac1e..d1a78b2 100644
--- 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 }
+
#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)

#define dynamic_dev_dbg(dev, 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_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__); \
+ __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
} while (0)

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