Skip to contentSkip to conversation

dynamic debug: cleanups + compile fix

PATCH In review Jason Baron 6 participants 16 messages Aug 25, 2011 – Aug 29, 2011

Subsystems (2)
Library code Dynamic debug
lore
Share & tools
lore

Jason Baron

author
#0 lore raw

cover dynamic debug: cleanups + compile fix

Hi,

This is a re-post of the last 3 patches, as requested by Greg:
https://lkml.org/lkml/2011/8/11/378

I've also added a compile fix that Randy Dunlap found in the -next tree.

Thanks,

-Jason

Jason Baron (4):
dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions
dynamic_debug: remove num_enabled accounting
dynamic_debug: use a single printk() to emit msgs
dynamic_debug: fix undefined reference to `__netdev_printk'

 include/linux/dynamic_debug.h |   64 ++++++++++++++++++---------------
 lib/dynamic_debug.c           |   77 ++++++++++++++++------------------------
 2 files changed, 66 insertions(+), 75 deletions(-)
Signature · 1 line
1.7.5.4
author
#1 lore raw

PATCH 1/4 dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions

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

Signed-off-by Jason Baron <[email protected]>
---
 include/linux/dynamic_debug.h |   64 ++++++++++++++++++++++------------------
 1 files changed, 35 insertions(+), 29 deletions(-)
include/linux/dynamic_debug.h +35 -29
index feaac1e..699f533 100644
@@ -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
Signature · 1 line
1.7.5.4
#2 lore raw

Re: [PATCH 1/4] dynamic_debug: consolidate repetitive struct _ddebug descriptor definitions

Quoted 12 lines · Jason Baron

+#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,                               \
+       }

A minor remark: macro arguments are usually surrounded by parentheses in a macro definition. That's not the case for "fmt" in the above.

Bart. ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±þG«�éÿŠ{ayºʇڙë,j­¢f£¢·hš�ï�êÿ‘êçz_è®(­éšŽŠÝ¢j"�ú¶m§ÿÿ¾«þG«�éÿ¢¸?™¨è­Ú&£ø§~�á¶iO•æ¬z·švØ^¶m§ÿÿà ÿ¶ìÿ¢¸?–I¥

author
#3 lore raw

PATCH 2/4 dynamic_debug: remove num_enabled accounting

In reply to Jason Baron

The num_enabled accounting isn't actually used anywhere - remove them.

Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)
lib/dynamic_debug.c +0 -7
index ee3b9ba..198d2af 100644
@@ -42,7 +42,6 @@ struct ddebug_table {
 	struct list_head link;
 	char *mod_name;
 	unsigned int num_ddebugs;
-	unsigned int num_enabled;
 	struct _ddebug *ddebugs;
 };
 

@@ -152,11 +151,6 @@ static void ddebug_change(const struct ddebug_query *query,
 			newflags = (dp->flags & mask) | flags;
 			if (newflags == dp->flags)
 				continue;
-
-			if (!newflags)
-				dt->num_enabled--;
-			else if (!dp->flags)
-				dt->num_enabled++;
 			dp->flags = newflags;
 			if (newflags)
 				dp->enabled = 1;

@@ -764,7 +758,6 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 	}
 	dt->mod_name = new_name;
 	dt->num_ddebugs = n;
-	dt->num_enabled = 0;
 	dt->ddebugs = tab;
 
 	mutex_lock(&ddebug_lock);
Signature · 1 line
1.7.5.4
author
#4 lore raw

PATCH 3/4 dynamic_debug: use a single printk() to emit msgs

In reply to Jason Baron

We were using KERN_CONT to combine msgs with their prefix. However, KERN_CONT is not smp safe, in the sense that it can interleave messages. This interleaving can result in printks coming out at the wrong loglevel. With the high frequency of printks, that dynamic debug can produce, this is not desirable.

Thus, make dynamic_emit_prefix(), fill a char buf[64], instead of doing a printk directly. If we enable printing out of function, module, line, or pid info, they are placed in this 64 byte buffer. In my testing 64 bytes was enough size to fulfill all requests. Even if its not, we can match up the printk itself to see where its from, so to me this is no big deal.

Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   66 +++++++++++++++++++++------------------------------
 1 files changed, 27 insertions(+), 39 deletions(-)
lib/dynamic_debug.c +27 -39
index 198d2af..1f11978 100644
@@ -422,52 +422,46 @@ static int ddebug_exec_query(char *query_string)
 	return 0;
 }
 
-static int dynamic_emit_prefix(const struct _ddebug *descriptor)
+#define PREFIX_SIZE 64
+#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
+
+static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 {
-	char tid[sizeof(int) + sizeof(int)/2 + 4];
-	char lineno[sizeof(int) + sizeof(int)/2];
+	int pos = 0;
 
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+	pos += snprintf(buf + pos, LEFT(pos), "%s", KERN_DEBUG);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
-			snprintf(tid, sizeof(tid), "%s", "<intr> ");
+			pos += snprintf(buf + pos, LEFT(pos), "%s ",
+						"<intr>");
 		else
-			snprintf(tid, sizeof(tid), "[%d] ",
-				 task_pid_vnr(current));
-	} else {
-		tid[0] = 0;
+			pos += snprintf(buf + pos, LEFT(pos), "[%d] ",
+						task_pid_vnr(current));
 	}
+	if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+		pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
 
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		snprintf(lineno, sizeof(lineno), "%d", descriptor->lineno);
-	else
-		lineno[0] = 0;
-
-	return printk(KERN_DEBUG "%s%s%s%s%s%s",
-		      tid,
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-		      descriptor->modname : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-		      ":" : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-		      descriptor->function : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-		      ":" : "",
-		      lineno);
+	return buf;
 }
 
 int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 {
 	va_list args;
 	int res;
+	struct va_format vaf;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
-	res = dynamic_emit_prefix(descriptor);
-	res += vprintk(fmt, args);
-
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	res = printk("%s%pV", dynamic_emit_prefix(descriptor, buf), &vaf);
 	va_end(args);
 
 	return res;

@@ -480,18 +474,15 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
 	struct va_format vaf;
 	va_list args;
 	int res;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
 	vaf.fmt = fmt;
 	vaf.va = &args;
-
-	res = dynamic_emit_prefix(descriptor);
-	res += __dev_printk(KERN_CONT, dev, &vaf);
-
+	res = __dev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
 	va_end(args);
 
 	return res;

@@ -504,18 +495,15 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 	struct va_format vaf;
 	va_list args;
 	int res;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
 	vaf.fmt = fmt;
 	vaf.va = &args;
-
-	res = dynamic_emit_prefix(descriptor);
-	res += __netdev_printk(KERN_CONT, dev, &vaf);
-
+	res = __netdev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
 	va_end(args);
 
 	return res;
Signature · 1 line
1.7.5.4
#5 lore raw

Re: [PATCH 3/4] dynamic_debug: use a single printk() to emit msgs

Quoted 42 lines · Jason Baron
lib/dynamic_debug.c +4 -1
-static int dynamic_emit_prefix(const struct _ddebug *descriptor)
+#define PREFIX_SIZE 64
+#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
+
+static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)

 {
-       char tid[sizeof(int) + sizeof(int)/2 + 4];
-       char lineno[sizeof(int) + sizeof(int)/2];
+       int pos = 0;

-       if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+       pos += snprintf(buf + pos, LEFT(pos), "%s", KERN_DEBUG);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
               if (in_interrupt())
-                       snprintf(tid, sizeof(tid), "%s", "<intr> ");
+                       pos += snprintf(buf + pos, LEFT(pos), "%s ",
+                                               "<intr>");
               else
-                       snprintf(tid, sizeof(tid), "[%d] ",
-                                task_pid_vnr(current));
-       } else {
-               tid[0] = 0;
+                       pos += snprintf(buf + pos, LEFT(pos), "[%d] ",
+                                               task_pid_vnr(current)); }
+       if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+               pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);

-       if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-               snprintf(lineno, sizeof(lineno), "%d", descriptor->lineno);
-       else
-               lineno[0] = 0;
-
-       return printk(KERN_DEBUG "%s%s%s%s%s%s",
-                     tid,
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-                     descriptor->modname : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?

Quoted 8 lines

-                     ":" : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-                     descriptor->function : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-                     ":" : "",
-                     lineno);
+       return buf;
 }

Shouldn't the new implementation guarantee that buf[] is '\0'-terminated if pos >= PREFIX_SIZE ?

Bart.

author
#6 lore raw

Re: [PATCH 3/4 re-post] dynamic_debug: use a single printk() to emit msgs

Quoted 55 lines · Bart Van Assche
Quoted 42 lines · Jason Baron
(quoted diff) +4 -1
-static int dynamic_emit_prefix(const struct _ddebug *descriptor)
+#define PREFIX_SIZE 64
+#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
+
+static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)

 {
-       char tid[sizeof(int) + sizeof(int)/2 + 4];
-       char lineno[sizeof(int) + sizeof(int)/2];
+       int pos = 0;

-       if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+       pos += snprintf(buf + pos, LEFT(pos), "%s", KERN_DEBUG);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
               if (in_interrupt())
-                       snprintf(tid, sizeof(tid), "%s", "<intr> ");
+                       pos += snprintf(buf + pos, LEFT(pos), "%s ",
+                                               "<intr>");
               else
-                       snprintf(tid, sizeof(tid), "[%d] ",
-                                task_pid_vnr(current));
-       } else {
-               tid[0] = 0;
+                       pos += snprintf(buf + pos, LEFT(pos), "[%d] ",
+                                               task_pid_vnr(current)); }
+       if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+               pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);

-       if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-               snprintf(lineno, sizeof(lineno), "%d", descriptor->lineno);
-       else
-               lineno[0] = 0;
-
-       return printk(KERN_DEBUG "%s%s%s%s%s%s",
-                     tid,
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-                     descriptor->modname : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?

Quoted 8 lines

-                     ":" : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-                     descriptor->function : "",
-                     (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-                     ":" : "",
-                     lineno);
+       return buf;
 }

Shouldn't the new implementation guarantee that buf[] is '\0'-terminated if pos >= PREFIX_SIZE ?

Quoted 1 line

Bart.

Yes, good catch. Here is a re-post.

thanks,

-Jason

We were using KERN_CONT to combine msgs with their prefix. However, KERN_CONT is not smp safe, in the sense that it can interleave messages. This interleaving can result in printks coming out at the wrong loglevel. With the high frequency of printks, that dynamic debug can produce, this is not desirable.

Thus, make dynamic_emit_prefix(), fill a char buf[64], instead of doing a printk directly. If we enable printing out of function, module, line, or pid info, they are placed in this 64 byte buffer. In my testing 64 bytes was enough size to fulfill all requests. Even if its not, we can match up the printk itself to see where its from, so to me this is no big deal.

Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   69 ++++++++++++++++++++++----------------------------
 1 files changed, 30 insertions(+), 39 deletions(-)
lib/dynamic_debug.c +30 -39
index 198d2af..f5def31 100644
@@ -422,52 +422,49 @@ static int ddebug_exec_query(char *query_string)
 	return 0;
 }
 
-static int dynamic_emit_prefix(const struct _ddebug *descriptor)
+#define PREFIX_SIZE 64
+#define LEFT(wrote) ((PREFIX_SIZE - wrote) > 0) ? (PREFIX_SIZE - wrote) : 0
+
+static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 {
-	char tid[sizeof(int) + sizeof(int)/2 + 4];
-	char lineno[sizeof(int) + sizeof(int)/2];
+	int pos = 0;
 
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+	pos += snprintf(buf + pos, LEFT(pos), "%s", KERN_DEBUG);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
-			snprintf(tid, sizeof(tid), "%s", "<intr> ");
+			pos += snprintf(buf + pos, LEFT(pos), "%s ",
+						"<intr>");
 		else
-			snprintf(tid, sizeof(tid), "[%d] ",
-				 task_pid_vnr(current));
-	} else {
-		tid[0] = 0;
+			pos += snprintf(buf + pos, LEFT(pos), "[%d] ",
+						task_pid_vnr(current));
 	}
+	if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+		pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
 
-	if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		snprintf(lineno, sizeof(lineno), "%d", descriptor->lineno);
-	else
-		lineno[0] = 0;
-
-	return printk(KERN_DEBUG "%s%s%s%s%s%s",
-		      tid,
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-		      descriptor->modname : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME) ?
-		      ":" : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-		      descriptor->function : "",
-		      (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME) ?
-		      ":" : "",
-		      lineno);
+	if (pos >= PREFIX_SIZE)
+		buf[PREFIX_SIZE - 1] = '\0';
+
+	return buf;
 }
 
 int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 {
 	va_list args;
 	int res;
+	struct va_format vaf;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
-	res = dynamic_emit_prefix(descriptor);
-	res += vprintk(fmt, args);
-
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	res = printk("%s%pV", dynamic_emit_prefix(descriptor, buf), &vaf);
 	va_end(args);
 
 	return res;

@@ -480,18 +477,15 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
 	struct va_format vaf;
 	va_list args;
 	int res;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
 	vaf.fmt = fmt;
 	vaf.va = &args;
-
-	res = dynamic_emit_prefix(descriptor);
-	res += __dev_printk(KERN_CONT, dev, &vaf);
-
+	res = __dev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
 	va_end(args);
 
 	return res;

@@ -504,18 +498,15 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 	struct va_format vaf;
 	va_list args;
 	int res;
+	char buf[PREFIX_SIZE];
 
 	BUG_ON(!descriptor);
 	BUG_ON(!fmt);
 
 	va_start(args, fmt);
-
 	vaf.fmt = fmt;
 	vaf.va = &args;
-
-	res = dynamic_emit_prefix(descriptor);
-	res += __netdev_printk(KERN_CONT, dev, &vaf);
-
+	res = __netdev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
 	va_end(args);
 
 	return res;
Signature · 1 line
1.7.5.4
maintainer
#7 lore raw

Re: [PATCH 3/4] dynamic_debug: use a single printk() to emit msgs

In reply to Jason Baron
Quoted 6 lines · Jason Baron

+       if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+               pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);

this inserts a space after lineno, but not otherwize.

I have patch to fix it, but maybe you want to fix it it here ?

lib/dynamic_debug.c +0 -0
index 9c8e133..122246f 100644
@@ -439,7 +439,9 @@ static char *dynamic_emit_prefix(const struct

_ddebug *desc, char *buf)
if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
- pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
+ pos += snprintf(buf + pos, LEFT(pos), "%d:", desc->lineno);
+ if (pos - strlen(KERN_DEBUG))
+ pos += snprintf(buf + pos, LEFT(pos), " ");

return buf;
}

Signature · 6 lines
1.7.4.4


also attached.
If it doesnt apply, or you defer, I'll resend once Greg's accepted
your whole set.

From ec75cd50ba89bed1f0ad74b89c9a43e87b2bd767 Mon Sep 17 00:00:00 2001
From: Jim Cromie <[email protected]>
Date: Tue, 2 Aug 2011 15:12:55 -0600
Subject: [PATCH 03/28] dynamic_debug: add space at end of non-null
dynamic-prefix

dynamic_emit_prefix() currently adds a space after line-number, but not otherwize. For readability, add space whenever any prefix (besides the "<7>" loglevel) has been written.

Signed-off-by Jim Cromie <[email protected]>
---
 lib/dynamic_debug.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
lib/dynamic_debug.c +3 -1
index 9c8e133..122246f 100644
@@ -439,7 +439,9 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
+		pos += snprintf(buf + pos, LEFT(pos), "%d:", desc->lineno);
+	if (pos - strlen(KERN_DEBUG))
+		pos += snprintf(buf + pos, LEFT(pos), " ");
 
 	return buf;
 }
Signature · 1 line
1.7.4.4
author
#8 lore raw

Re: [PATCH 3/4] dynamic_debug: use a single printk() to emit msgs

Quoted 9 lines · Jim Cromie
Quoted 6 lines · Jason Baron

+       if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->modname);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+               pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
+       if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+               pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);

Quoted 4 lines

this inserts a space after lineno, but not otherwize.

I have patch to fix it, but maybe you want to fix it it here ?

Hi Jim,

Thanks for your fix...since Greg hasn't taken the series yet...I guess I'll re-spin to include this fix. Probably wouldn't send it out until tomorrow due to my test systems being down atm...

thanks,

-Jason

Quoted 57 lines
lib/dynamic_debug.c +3 -1
index 9c8e133..122246f 100644
@@ -439,7 +439,9 @@ static char *dynamic_emit_prefix(const struct
 _ddebug *desc, char *buf)
 	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
+		pos += snprintf(buf + pos, LEFT(pos), "%d:", desc->lineno);
+	if (pos - strlen(KERN_DEBUG))
+		pos += snprintf(buf + pos, LEFT(pos), " ");
 
 	return buf;

}

Signature · 21 lines
1.7.4.4


also attached.
If it doesnt apply, or you defer, I'll resend once Greg's accepted
your whole set.

From ec75cd50ba89bed1f0ad74b89c9a43e87b2bd767 Mon Sep 17 00:00:00 2001
From: Jim Cromie <[email protected]>
Date: Tue, 2 Aug 2011 15:12:55 -0600
Subject: [PATCH 03/28] dynamic_debug: add space at end of non-null
 dynamic-prefix

dynamic_emit_prefix() currently adds a space after line-number,
but not otherwize.  For readability, add space whenever any prefix
(besides the "<7>" loglevel) has been written.

Signed-off-by: Jim Cromie <[email protected]>
---
 lib/dynamic_debug.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
lib/dynamic_debug.c +3 -1
index 9c8e133..122246f 100644
@@ -439,7 +439,9 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, LEFT(pos), "%s:", desc->function);
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		pos += snprintf(buf + pos, LEFT(pos), "%d ", desc->lineno);
+		pos += snprintf(buf + pos, LEFT(pos), "%d:", desc->lineno);
+	if (pos - strlen(KERN_DEBUG))
+		pos += snprintf(buf + pos, LEFT(pos), " ");
 
 	return buf;
 }
Signature · 1 line
1.7.4.4
author
#9 lore raw

PATCH 4/4 dynamic_debug: fix undefined reference to `__netdev_printk'

In reply to Jason Baron

Dynamic debug recently added a call to __netdev_printk(). However, when CONFIG_NET is not set, we get the following error:

lib/built-in.o: In function `__dynamic_netdev_dbg':
(.text+0x9fda): undefined reference to `__netdev_printk'

Fix this by making the call to __netdev_printk() contingent upon CONFIG_NET. We could have fixed this by defining __netdev_printk() to a 'no-op' in the !CONFIG_NET case. However, this is not consistent with how the networking layer uses netdev_printk(). For example, when CONFIG_NET is not set, netdev_printk() does not have a 'no-op' definition defined.

Signed-off-by Jason Baron <[email protected]>
Acked-by Randy Dunlap <[email protected]>
---
 lib/dynamic_debug.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
lib/dynamic_debug.c +4 -0
index 1f11978..bda4036 100644
@@ -489,6 +489,8 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
 }
 EXPORT_SYMBOL(__dynamic_dev_dbg);
 
+#ifdef CONFIG_NET
+
 int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 		      const struct net_device *dev, const char *fmt, ...)
 {

@@ -510,6 +512,8 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 }
 EXPORT_SYMBOL(__dynamic_netdev_dbg);
 
+#endif
+
 static __initdata char ddebug_setup_string[1024];
 static __init int ddebug_setup_query(char *str)
 {
Signature · 1 line
1.7.5.4
author
#11 lore raw
Quoted 10 lines · Greg KH
Quoted 6 lines · Jason Baron

Hi,

This is a re-post of the last 3 patches, as requested by Greg:
https://lkml.org/lkml/2011/8/11/378

I've also added a compile fix that Randy Dunlap found in the -next tree.

Care to redo this based on the comments received so far?

Hi,

I re-posted 3/4 based on the comments already. The comment on patch #1 regarding adding macro parenthesis, I don't believe makes any functional change...do you still need a re-post the series?

Thanks,

-Jason

#12 lore raw
Quoted 9 lines · Jason Baron
Quoted 8 lines · Greg KH
Quoted 6 lines · Jason Baron

Hi,

This is a re-post of the last 3 patches, as requested by Greg:
https://lkml.org/lkml/2011/8/11/378

I've also added a compile fix that Randy Dunlap found in the -next tree.

Quoted 8 lines
Quoted 2 lines

Care to redo this based on the comments received so far?

Hi,

I re-posted 3/4 based on the comments already. The comment on patch #1 regarding adding macro parenthesis, I don't believe makes any functional change...do you still need a re-post the series?

Yes, please fix patch #1 adding the needed parenthesis and resend the whole series.

thanks,

greg k-h

#13 lore raw
Quoted 13 lines · Greg KH
Quoted 12 lines · Jason Baron
Quoted 10 lines · Greg KH
Quoted 6 lines · Jason Baron

Hi,

This is a re-post of the last 3 patches, as requested by Greg:
https://lkml.org/lkml/2011/8/11/378

I've also added a compile fix that Randy Dunlap found in the -next tree.

Care to redo this based on the comments received so far?

Quoted 8 lines
Quoted 5 lines

Hi,

I re-posted 3/4 based on the comments already. The comment on patch #1 regarding adding macro parenthesis, I don't believe makes any functional change...do you still need a re-post the series?

Yes, please fix patch #1 adding the needed parenthesis and resend the whole series.

The parentheses aren't need.

A macro like
#define foo(a, b) a = b

that only uses each argument once without doing arithmetic gains nothing from parentheses.

Just like
#define true 1
gains nothing from parenthesis either.

#14 lore raw
Quoted 28 lines · Joe Perches
Quoted 13 lines · Greg KH
Quoted 12 lines · Jason Baron
Quoted 10 lines · Greg KH
Quoted 6 lines · Jason Baron

Hi,

This is a re-post of the last 3 patches, as requested by Greg:
https://lkml.org/lkml/2011/8/11/378

I've also added a compile fix that Randy Dunlap found in the -next tree.

Care to redo this based on the comments received so far?

Quoted 8 lines
Quoted 5 lines

Hi,

I re-posted 3/4 based on the comments already. The comment on patch #1 regarding adding macro parenthesis, I don't believe makes any functional change...do you still need a re-post the series?

Yes, please fix patch #1 adding the needed parenthesis and resend the whole series.

The parentheses aren't need.

A macro like
#define foo(a, b) a = b

Quoted 6 lines

that only uses each argument once without doing arithmetic gains nothing from parentheses.

Just like
#define true 1
gains nothing from parenthesis either.

I don't think that is the case here, right?

greg k-h

#15 lore raw
Quoted 9 lines · Greg KH
Quoted 7 lines · Joe Perches
Quoted 4 lines · Greg KH
Quoted 4 lines
Quoted 1 line

Care to redo this based on the comments received so far?

I re-posted 3/4 based on the comments already. The comment on patch #1 regarding adding macro parenthesis, I don't believe makes any functional change...do you still need a re-post the series?

#define true 1
gains nothing from parenthesis either.

I don't think that is the case here, right?

Nope.

It is the case, the parentheses are not necessary.