lkml.org 
[lkml]   [2011]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 09/11 re-post take #2] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions

    Hi Joe,

    On Thu, Aug 11, 2011 at 11:39:24PM -0700, Joe Perches wrote:
    > 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?

    yes, that would be better than what we have now.

    >
    > #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?

    right, but I'm leaving this to a subsequent patch.

    >
    > > #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)

    ok.

    >
    > Lastly, are the unlikely()s really useful?
    >

    I think the unlikely's make sense, since we don't expect these to be
    enabled by default.

    So, I've re-spun the patch, with the above comments (below). Thanks!

    -Jason

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

    Signed-off-by: Jason Baron <jbaron@redhat.com>
    ---
    include/linux/dynamic_debug.h | 64 ++++++++++++++++++++++------------------
    1 files changed, 35 insertions(+), 29 deletions(-)

    diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
    index feaac1e..699f533 100644
    --- a/include/linux/dynamic_debug.h
    +++ b/include/linux/dynamic_debug.h
    @@ -54,35 +54,41 @@ extern int __dynamic_netdev_dbg(struct _ddebug *descriptor,
    const char *fmt, ...)
    __attribute__ ((format (printf, 3, 4)));

    -#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 }; \
    - if (unlikely(descriptor.enabled)) \
    - __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 }; \
    - if (unlikely(descriptor.enabled)) \
    - __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)) \
    - __dynamic_netdev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__);\
    - } while (0)
    +#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt) \
    + static struct _ddebug __used __aligned(8) \
    + __attribute__((section("__verbose"))) name = { \
    + .modname = KBUILD_MODNAME, \
    + .function = __func__, \
    + .filename = __FILE__, \
    + .format = fmt, \
    + .lineno = __LINE__, \
    + .flags = _DPRINTK_FLAGS_DEFAULT, \
    + .enabled = false, \
    + }
    +
    +#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)
    +
    +#define dynamic_dev_dbg(dev, fmt, ...) \
    +do { \
    + DECLARE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
    + if (unlikely(descriptor.enabled)) \
    + __dynamic_dev_dbg(&descriptor, dev, fmt, \
    + ##__VA_ARGS__); \
    +} while (0)
    +
    +#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)

    #else

    --
    1.7.5.4


    \
     
     \ /
      Last update: 2011-08-15 22:47    [W:6.478 / U:0.544 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site