| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20110815204438.GA7157@redhat.com>
Date: Mon, 15 Aug 2011 16:44:38 -0400
From: Jason Baron <jbaron@...hat.com>
To: Joe Perches <joe@...ches.com>
Cc: gregkh@...e.de, jim.cromie@...il.com, bvanassche@....org,
linux-kernel@...r.kernel.org
Subject: Re: [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@...hat.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists