2010-12-03 14:24:19

by Roman Fietze

[permalink] [raw]
Subject: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [0/3]

Hello Jason,

There will be following two small patches adding dynamic printk
support for print_hex_dump in two steps shortly.

The first patch will add a pr_ like wrapper as it already exists for
printk(<level>,..., e.g. pr_info. The second patch will finally add a
dynamic debug wrapper simply duplicating your work for printk debug
output.

These patches are against the current head of the torvald repos as of
today. If somebody asks for a patch against the last released version
I can provide those as well.


Roman

--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de


2010-12-03 14:26:30

by Roman Fietze

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [1/2]


>From 9990f3b651adad147a40bd4ce6289182e3b7da74 Mon Sep 17 00:00:00 2001
From: Roman Fietze <[email protected]>
Date: Fri, 3 Dec 2010 14:06:56 +0100
Subject: [PATCH 1/2] kernel.h: add pr_<level>_hex_dump macros

Add macros e.g. named pr_<level>_hex_dump calling print_hex_dump with
the approriate level, with level beeing emerg, alert, ..., debug. This
is similiar to the functions starting with "pr_".

The parameters for those macros are the same as for print_hex_dump
excluding the level.

Signed-off-by: Roman Fietze <[email protected]>
---
include/linux/printk.h | 38 ++++++++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index b772ca5..77903f1 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -179,6 +179,44 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
#define pr_cont(fmt, ...) \
printk(KERN_CONT fmt, ##__VA_ARGS__)

+#define pr_emerg_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_EMERG, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_alert_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_ALERT, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_crit_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_CRIT, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_err_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_ERR, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_warning_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_WARNING, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_warn_hex_dump pr_warning_hex_dump
+#define pr_notice_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_NOTICE, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_info_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_INFO, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_cont_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_CONT, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
+
/* pr_devel() should produce zero code unless DEBUG is defined */
#ifdef DEBUG
#define pr_devel(fmt, ...) \
--
1.7.3.2



--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de

2010-12-03 14:28:26

by Roman Fietze

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [2/2]

Sorry for the first, wrong subject line. The index field shoudl have
been reading [0/2].


>From 068faf208310a0a72194667143a5a9ca9beabdde Mon Sep 17 00:00:00 2001
From: Roman Fietze <[email protected]>
Date: Fri, 3 Dec 2010 15:00:43 +0100
Subject: [PATCH 2/2] dynamic printk: add support for pr_debug_hex_dump

Use dynamic printk wrapper to support turning pr_debug_hex_dump on and
off similar to pr_debug using the dynamic debug sysfs control file.

Signed-off-by: Roman Fietze <[email protected]>
---
include/linux/dynamic_debug.h | 22 ++++++++++++++++++++++
include/linux/printk.h | 16 ++++++++++++----
2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index a90b389..0610e74 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -74,6 +74,24 @@ do_printk: \
out: ; \
} while (0)

+#define dynamic_pr_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ do { \
+ __label__ do_hex_dump; \
+ __label__ out; \
+ static struct _ddebug descriptor \
+ __used \
+ __attribute__((section("__verbose"), aligned(8))) = { \
+ KBUILD_MODNAME, __func__, __FILE__, prfx_str, __LINE__, \
+ _DPRINTK_FLAGS_DEFAULT }; \
+ JUMP_LABEL(&descriptor.enabled, do_hex_dump); \
+ goto out; \
+ do_hex_dump: \
+ print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, \
+ rowsz, grpsz, buf, len, ascii); \
+ out: ; \
+ } while (0)
+
#else

static inline int ddebug_remove_module(const char *mod)
@@ -85,6 +103,10 @@ static inline int ddebug_remove_module(const char *mod)
do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
#define dynamic_dev_dbg(dev, fmt, ...) \
do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dynamic_pr_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ do { if (0) print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii); } while (0)
#endif

#endif
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 77903f1..5bf4483 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -212,10 +212,6 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
buf, len, ascii) \
print_hex_dump(KERN_CONT, prfx_str, prfx_type, rowsz, grpsz, \
buf, len, ascii)
-#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
- buf, len, ascii) \
- print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
- buf, len, ascii)

/* pr_devel() should produce zero code unless DEBUG is defined */
#ifdef DEBUG
@@ -230,13 +226,25 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
#if defined(DEBUG)
#define pr_debug(fmt, ...) \
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
#elif defined(CONFIG_DYNAMIC_DEBUG)
/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
#define pr_debug(fmt, ...) \
dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ dynamic_pr_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii)
#else
#define pr_debug(fmt, ...) \
({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
+#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii) \
+ do { if (0) print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
+ buf, len, ascii); } while (0)
#endif

/*
--
1.7.3.2




--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de

2010-12-03 20:35:15

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [1/2]

On Fri, 2010-12-03 at 15:19 +0100, Roman Fietze wrote:
> >From 9990f3b651adad147a40bd4ce6289182e3b7da74 Mon Sep 17 00:00:00 2001
> From: Roman Fietze <[email protected]>
> Date: Fri, 3 Dec 2010 14:06:56 +0100
> Subject: [PATCH 1/2] kernel.h: add pr_<level>_hex_dump macros
> Add macros e.g. named pr_<level>_hex_dump calling print_hex_dump with
> the approriate level, with level beeing emerg, alert, ..., debug. This
> is similiar to the functions starting with "pr_".
[]
> diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
> @@ -179,6 +179,44 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> #define pr_cont(fmt, ...) \
> printk(KERN_CONT fmt, ##__VA_ARGS__)
>
> +#define pr_emerg_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \
> + print_hex_dump(KERN_EMERG, prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii)

Please use full argument names

#define pr_emerg_hex_dump(prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii) \
print_hex_dump(KERN_EMERG, prefix_str, prefix_type,
rowsize, groupsize, buf, len, ascii)

etc...

Perhaps prefix_str should be pr_fmt(prefix_str)

> +#define pr_warning_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \
> + print_hex_dump(KERN_WARNING, prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii)

As these macros are not yet in use anywhere, I think
that pr_warning_hex_dump should not be defined and
that pr_warn_hex_dump is sufficient and compatible
with the current use styles in dev_<level> and others.

[]

> +#define pr_cont_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \
> + print_hex_dump(KERN_CONT, prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii)

pr_cont_hex_dump should not be defined.

2010-12-03 20:37:04

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [2/2]

On Fri, 2010-12-03 at 15:21 +0100, Roman Fietze wrote:
> Subject: [PATCH 2/2] dynamic printk: add support for pr_debug_hex_dump
> Use dynamic printk wrapper to support turning pr_debug_hex_dump on and
> off similar to pr_debug using the dynamic debug sysfs control file.
>
> Signed-off-by: Roman Fietze <[email protected]>
[]
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index a90b389..0610e74 100644
> @@ -74,6 +74,24 @@ do_printk: \
> out: ; \
> } while (0)
>
> +#define dynamic_pr_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \

I think this should be dynamic_pr_debug_hex_dump and
I think the arguments should be:

#define dynamic_pr_debug_hex_dump(prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii) \

to mirror the print_hex_dump args.

> +#define dynamic_pr_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \
> + do { if (0) print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii); } while (0)
> #endif

here too.

But because this doesn't fit sensibly on a single line,
perhaps this should now be:

#define dynamic_pr_debug_hex_dump(prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii) \
{ \
if (0) \
print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii); \
}

> @@ -230,13 +226,25 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> #if defined(DEBUG)
> #define pr_debug(fmt, ...) \
> printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_debug_hex_dump(prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii) \
> + print_hex_dump(KERN_DEBUG, prfx_str, prfx_type, rowsz, grpsz, \
> + buf, len, ascii)

Please use the same argument names as print_hex_dump

2010-12-06 05:58:58

by Roman Fietze

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [2/2]

Hello Joe,

On Friday 03 December 2010 21:37:00 Joe Perches wrote:

> I think this should be dynamic_pr_debug_hex_dump and

No problem, I see your point.

> ...
> to mirror the print_hex_dump args.

Accepted. I shortened the names to shorten the macros (numer of
lines), but changing this back to the old names isn't a problem.

No longer offering the warning-version and just the warn-version is no
problem as well.

I will incoroprate your proposals into a new set of patches and offer
theme here, when I got Jason's response ... or a timeout.


Roman

--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de

2010-12-06 06:04:50

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Proposal to add dynamic debug feature for print_hex_dump [2/2]

On Mon, 2010-12-06 at 06:58 +0100, Roman Fietze wrote:
> I will incorporate your proposals into a new set of patches and
> offer them here, when I got Jason's response ... or a timeout.

Thanks Roman.

I just submitted several other printk.h patches.
Perhaps you might rebase these on top of those?