convert dynamic_debug to use jump labels

The stringify_in_c() macro may not be included. Make the dependency explicit.

Cc Benjamin Herrenschmidt <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Michael Ellerman <[email protected]>

Cc [email protected]

Signed-off-by Jason Baron <[email protected]>

The jump table can reference text found in an __exit section. Thus, instead of discarding it at build/link time, include EXIT_TEXT as part of __init and release it at system boot time.

Without this patch the link fails with:

.exit.text' referenced in section __jump_table' of xxx.o:
defined in discarded section `.exit.text' of xxx.o

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Signed-off-by Jason Baron <[email protected]>

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

   text    data     bss     dec     hex filename
8194852 4879776  925696 14000324  d5a0c4 vmlinux.pre
8187337 4960224  925696 14073257  d6bda9 vmlinux.post

include/linux/dynamic_debug.h +0 -0

@@ -60,7 +70,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
  			  const struct net_device *dev,
  			  const char *fmt, ...);

include/linux/dynamic_debug.h +1 -1

-#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)		\
+#define DEFINE_DYNAMIC_DEBUG_METADATA_KEY(name, fmt, key, init)	\

  static struct _ddebug  __aligned(8) \
  __attribute__((section("__verbose"))) name = { \

  .modname = KBUILD_MODNAME, \

Quoted 9 lines · Jason Baron

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

text data bss dec hex filename
8194852 4879776 925696 14000324 d5a0c4 vmlinux.pre
8187337 4960224 925696 14073257 d6bda9 vmlinux.post

Maybe to get some of that space back on the 32 bit builds the 8 byte alignment can be relaxed.

include/linux/dynamic_debug.h

Quoted 1 line

include/linux/dynamic_debug.h +0 -0

[]

include/linux/dynamic_debug.h struct _ddebug { · line 46

Quoted 11 lines

include/linux/dynamic_debug.h +6 -0

@@ -33,6 +37,12 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_DEFAULT 0
 #endif
 	unsigned int flags:8;
+#ifdef HAVE_JUMP_LABEL
+	union {
+		struct static_key_true dd_key_true;
+		struct static_key_false dd_key_false;
+	} key;
+#endif
 } __attribute__((aligned(8)));

Couldn't this be:

} __aligned(__alignof__(void *));

include/linux/dynamic_debug.h void __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 75

Quoted 8 lines

include/linux/dynamic_debug.h +1 -1

@@ -60,7 +70,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
 			  const struct net_device *dev,
 			  const char *fmt, ...);
 
-#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)		\
+#define DEFINE_DYNAMIC_DEBUG_METADATA_KEY(name, fmt, key, init)	\
 	static struct _ddebug  __aligned(8)			\
 	__attribute__((section("__verbose"))) name = {		\

And this shouldn't need the __aligned as it's already in the structure definition.

Maybe:

	static struct _ddebug __section(__verbose) name = {

Quoted 1 line

include/linux/dynamic_debug.h +0 -0

 		.modname = KBUILD_MODNAME,			\

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

...

Quoted 8 lines · On Wed, 6 Jul 2016 17:42:36 -0400 Jason Baron <[email protected]> wrote:

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

...

include/linux/dynamic_debug.h void __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 108

Quoted 10 lines

include/linux/dynamic_debug.h +7 -0

+#ifdef HAVE_JUMP_LABEL
+
+#define dd_key_init(key, init) key = (init)
 
...
 
+#else
+
+#define dd_key_init(key, init)
+

umm, lazy. One is an lval and returns a value and the other does neither. Lack of parenthesization in the first version doubtless exposes various horrors.

Care to do something more robust and conventional here? Presumably use symmetrical do{}while(0) things, neither of which is an lval, both of which don't return anything.

Quoted 24 lines · Andrew Morton

Quoted 8 lines · On Wed, 6 Jul 2016 17:42:36 -0400 Jason Baron <[email protected]> wrote:

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

...

include/linux/dynamic_debug.h void __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 108

Quoted 10 lines

include/linux/dynamic_debug.h +7 -0

+#ifdef HAVE_JUMP_LABEL
+
+#define dd_key_init(key, init) key = (init)
 
...
 
+#else
+
+#define dd_key_init(key, init)
+

umm, lazy. One is an lval and returns a value and the other does neither. Lack of parenthesization in the first version doubtless exposes various horrors.

Quoted 4 lines

Care to do something more robust and conventional here? Presumably use symmetrical do{}while(0) things, neither of which is an lval, both of which don't return anything.

Hi,

The 'dd_key_init()' macro is being used here to help initialize the 'key' field in the 'struct _ddebug', and its not being used as a statement.

In the 'HAVE_JUMP_LABEL' case, we are initializing the 'key' field, while in the not-'HAVE_JUMP_LABEL' case, the 'key' field is simply not present in the structure (to conserve space).

Quoted 31 lines · On Mon, 11 Jul 2016 09:18:21 -0400 Jason Baron <[email protected]> wrote:

Quoted 24 lines · Andrew Morton

Quoted 8 lines · On Wed, 6 Jul 2016 17:42:36 -0400 Jason Baron <[email protected]> wrote:

Although dynamic debug is often only used for debug builds, sometimes its enabled for production builds as well. Minimize its impact by using jump labels. This reduces the text section by 7000+ bytes in the kernel image below. It does increase data, but this should only be referenced when changing the direction of the branches, and hence usually not in cache.

...

include/linux/dynamic_debug.h void __dynamic_netdev_dbg(struct _ddebug *descriptor, · line 108

Quoted 10 lines

include/linux/dynamic_debug.h +7 -0

+#ifdef HAVE_JUMP_LABEL
+
+#define dd_key_init(key, init) key = (init)
 
...
 
+#else
+
+#define dd_key_init(key, init)
+

umm, lazy. One is an lval and returns a value and the other does neither. Lack of parenthesization in the first version doubtless exposes various horrors.

Quoted 4 lines

Care to do something more robust and conventional here? Presumably use symmetrical do{}while(0) things, neither of which is an lval, both of which don't return anything.

Hi,

Quoted 7 lines

The 'dd_key_init()' macro is being used here to help initialize the 'key' field in the 'struct _ddebug', and its not being used as a statement.

In the 'HAVE_JUMP_LABEL' case, we are initializing the 'key' field, while in the not-'HAVE_JUMP_LABEL' case, the 'key' field is simply not present in the structure (to conserve space).

Well yeah. And it's doing it wrongly, isn't it?

: @@ -68,13 +78,51 @@ void __dynamic_netdev_dbg(struct _ddebug

:  		.filename = __FILE__,				\
:  		.format = (fmt),				\
:  		.lineno = __LINE__,				\

: - .flags = _DPRINTK_FLAGS_DEFAULT, \
: + .flags = _DPRINTK_FLAGS_DEFAULT, \
: + dd_key_init(key, init) \
: }
:
: +#ifdef HAVE_JUMP_LABEL
: +
: +#define dd_key_init(key, init) key = (init)

Shouldn't it be ".key = (init)"?

Anyway, it's odd-looking. I guess something like

#define DD_KEY_INIT(init) .key = (init)

would be more idiomatic.