On Sun, 2012-11-18 at 11:47 +0200, Vladimir Kondratiev wrote:
> I see 2 drivers that use dynamic prefix:
>
> - mentioned above drivers/tty/ipwireless/hardware.c
> - drivers/isdn/hardware/mISDN/
>
> I see several options:
>
> 1) require prefix_str to be constant. Patch code mentioned. For example,
> code above may reads:
>
> static void dump_data_bytes(const char *type, const unsigned char *data,
> unsigned length)
> {
> pr_debug(IPWIRELESS_PCCARD_NAME ": %s %s\n",
> type, data_type(data, length));
> print_hex_dump_bytes(IPWIRELESS_PCCARD_NAME, 0, (void *)data,
> length < DUMP_MAX_BYTES ? length : DUMP_MAX_BYTES);
> }
>
> 2) introduce fixed format string in DEFINE_DYNAMIC_DEBUG_METADATA, like
>
> DEFINE_DYNAMIC_DEBUG_METADATA((descriptor, "hexdump");
>
> 3) remove print_hex_dump_bytes from this patch
>
> I think 1) is the best, opinions?
Another option could be testing __builtin_constant_p(prefix)
On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
> Another option could be testing __builtin_constant_p(prefix)
You mean something like below? Yes, it will work as well.
Pro: don't need to change existing code
(drop [PATCH 2/2] dynamic_debug: use constant format in print_hex_dump_bytes())
Cons: format in dynamic metadata will be useless
What looks better?
---
#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
do { \
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
__builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
print_hex_dump(KERN_DEBUG, prefix_str, \
prefix_type, rowsize, groupsize, \
buf, len, ascii); \
} while (0)
On Sunday, November 18, 2012 05:43:07 PM Vladimir Kondratiev wrote:
> On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
> > Another option could be testing __builtin_constant_p(prefix)
>
> You mean something like below? Yes, it will work as well.
>
> Pro: don't need to change existing code
> (drop [PATCH 2/2] dynamic_debug: use constant format in
> print_hex_dump_bytes())
>
> Cons: format in dynamic metadata will be useless
>
> What looks better?
>
> ---
> #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
> groupsize, buf, len, ascii) \
> do { \
> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
> __builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
> print_hex_dump(KERN_DEBUG, prefix_str, \
> prefix_type, rowsize, groupsize, \
> buf, len, ascii); \
> } while (0)
In case this option wins, patch follows. There is no need for
2-nd one to fix existing drivers.
>From 42bea6be2b3899eac1ed3f48eb955a4d83960cf5 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev <[email protected]>
Date: Sun, 18 Nov 2012 18:56:29 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
Documentation/dynamic-debug-howto.txt | 15 +++++++++++++--
include/linux/dynamic_debug.h | 11 +++++++++++
include/linux/printk.h | 17 +++++++++++++++++
lib/hexdump.c | 4 +++-
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 6e16849..72322c6 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,16 @@ This document describes how to use the dynamic debug (dyndbg) feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
+
+For print_hex_dump_debug()/print_hex_dump_bytes(), format string is
+its 'prefix_str' argument, if it is constant string; or "hexdump"
+in case 'prefix_str' is build dynamically.
Dynamic debug has even more useful features:
@@ -202,6 +210,9 @@ The flags are:
t Include thread ID in messages not generated from interrupt context
_ No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..2fe93b2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,17 @@ do { \
##__VA_ARGS__); \
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
+ __builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii); \
+} while (0)
+
#else
#include <linux/string.h>
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else
extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
@@ -336,4 +341,16 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
#endif
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii)
+#else
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii)
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
+
#endif
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 6540d65..3f0494c 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -227,6 +227,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
}
EXPORT_SYMBOL(print_hex_dump);
+#if !defined(CONFIG_DYNAMIC_DEBUG)
/**
* print_hex_dump_bytes - shorthand form of print_hex_dump() with default params
* @prefix_str: string to prefix each line with;
@@ -246,4 +247,5 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
buf, len, true);
}
EXPORT_SYMBOL(print_hex_dump_bytes);
-#endif
+#endif /* !defined(CONFIG_DYNAMIC_DEBUG) */
+#endif /* defined(CONFIG_PRINTK) */
--
1.7.10.4
On Sun, 2012-11-18 at 19:01 +0200, Vladimir Kondratiev wrote:
> In case this option wins, patch follows. There is no need for
> 2-nd one to fix existing drivers.
Thanks Vladimir.
On Sun, Nov 18, 2012 at 10:35:40AM -0800, Joe Perches wrote:
> On Sun, 2012-11-18 at 19:01 +0200, Vladimir Kondratiev wrote:
> > In case this option wins, patch follows. There is no need for
> > 2-nd one to fix existing drivers.
>
> Thanks Vladimir.
>
>
Yes, thanks. I've pulled in this patch, and posted a dynamic debug tree
at: git clone git://github.com/jibaron/ddebug.git
I will push the collected set of patches to Greg.
I also added a couple of patches there to convert dynamic debug to use
'jump labels', which shows good results.
Thanks,
-Jason