2012-05-17 20:02:40

by Joe Perches

[permalink] [raw]
Subject: [RFC PATCH] dynamic_debug: Remove __used attribute from metadata

The __used attribute in the declaration of the
dynamic_debug metadata stops the compiler from
optimizing and eliminating constant tests and
the metadata declaration used in things like:

#define DEBUG_LEVEL 0
if (DEBUG_LEVEL > 1)
pr_debug("foo...");

This is a common construct for debugging macros
with a constant "level" test.

When dynamic_debug is not configured, this is
pr_debug and format string is eliminated unless
DEBUG_LEVEL is greater than 1.

Remove the unnecessary __used attribute so the
even the dynamic_debug use of pr_debug can be
appropriately optimized away completely.

Signed-off-by: Joe Perches <[email protected]>
---
include/linux/dynamic_debug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7e3c53a..85cee11 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -58,7 +58,7 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
const char *fmt, ...);

#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
- static struct _ddebug __used __aligned(8) \
+ static struct _ddebug __aligned(8) \
__attribute__((section("__verbose"))) name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \


2012-05-23 18:01:01

by Jim Cromie

[permalink] [raw]
Subject: Re: [RFC PATCH] dynamic_debug: Remove __used attribute from metadata

On Thu, May 17, 2012 at 2:02 PM, Joe Perches <[email protected]> wrote:
> The __used attribute in the declaration of the
> dynamic_debug metadata stops the compiler from
> optimizing and eliminating constant tests and
> the metadata declaration used in things like:
>
> ? #define DEBUG_LEVEL 0
> ? if (DEBUG_LEVEL > 1)
> ? ? ? ?pr_debug("foo...");
>
> This is a common construct for debugging macros
> with a constant "level" test.
>
> When dynamic_debug is not configured, this is
> pr_debug and format string is eliminated unless
> DEBUG_LEVEL is greater than 1.
>
> Remove the unnecessary __used attribute so the
> even the dynamic_debug use of pr_debug can be
> appropriately optimized away completely.
>


Im a bit puzzled - the __used attr is in the
#if defined(CONFIG_DYNAMIC_DEBUG) branch.

If its not config'd, the METADATA is not compiled,
and this should have no effect.
Did you mean enabled instead of configured ?

FWIW, removing __used causes no harm here,
ie it doesnt break dynamic-debug facility.

Still, Id like to hear from Jason, he wrote it.

2012-05-23 18:10:46

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC PATCH] dynamic_debug: Remove __used attribute from metadata

On Wed, 2012-05-23 at 12:00 -0600, Jim Cromie wrote:
> On Thu, May 17, 2012 at 2:02 PM, Joe Perches <[email protected]> wrote:
> > The __used attribute in the declaration of the
> > dynamic_debug metadata stops the compiler from
> > optimizing and eliminating constant tests and
> > the metadata declaration used in things like:
> >
> > #define DEBUG_LEVEL 0
> > if (DEBUG_LEVEL > 1)
> > pr_debug("foo...");
> >
> > This is a common construct for debugging macros
> > with a constant "level" test.
> >
> > When dynamic_debug is not configured, this is
> > pr_debug and format string is eliminated unless
> > DEBUG_LEVEL is greater than 1.
> >
> > Remove the unnecessary __used attribute so the
> > even the dynamic_debug use of pr_debug can be
> > appropriately optimized away completely.
> >
>
>
> Im a bit puzzled - the __used attr is in the
> #if defined(CONFIG_DYNAMIC_DEBUG) branch.

Correct, the optimization is for CONFIG_DYNAMIC_DEBUG
enabled.

> If its not config'd, the METADATA is not compiled,
> and this should have no effect.
> Did you mean enabled instead of configured ?

Nope.

if (0)
dynamic_pr_debug("foo %u\n", bar);

will not be optimized away completely because
dynamic_pr_debug uses
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);
and that DEFINE includes __used

so even though the compiler knows that the branch
can not be taken, it still declares the known
unnecessary metadata.

2012-05-23 19:06:10

by Jason Baron

[permalink] [raw]
Subject: Re: [RFC PATCH] dynamic_debug: Remove __used attribute from metadata

On Wed, May 23, 2012 at 12:00:29PM -0600, Jim Cromie wrote:
> On Thu, May 17, 2012 at 2:02 PM, Joe Perches <[email protected]> wrote:
> > The __used attribute in the declaration of the
> > dynamic_debug metadata stops the compiler from
> > optimizing and eliminating constant tests and
> > the metadata declaration used in things like:
> >
> > ? #define DEBUG_LEVEL 0
> > ? if (DEBUG_LEVEL > 1)
> > ? ? ? ?pr_debug("foo...");
> >
> > This is a common construct for debugging macros
> > with a constant "level" test.
> >
> > When dynamic_debug is not configured, this is
> > pr_debug and format string is eliminated unless
> > DEBUG_LEVEL is greater than 1.
> >
> > Remove the unnecessary __used attribute so the
> > even the dynamic_debug use of pr_debug can be
> > appropriately optimized away completely.
> >
>
>
> Im a bit puzzled - the __used attr is in the
> #if defined(CONFIG_DYNAMIC_DEBUG) branch.
>
> If its not config'd, the METADATA is not compiled,
> and this should have no effect.
> Did you mean enabled instead of configured ?
>
> FWIW, removing __used causes no harm here,
> ie it doesnt break dynamic-debug facility.
>
> Still, Id like to hear from Jason, he wrote it.

Yeah, can't recall atm why its there. I agree that since the call sites
reference the '_ddebug' structure it should be unecessary. So feel free
to add:

Acked-by: Jason Baron <[email protected]>

Thanks,

-Jason