Re: [PATCH 1/7] dynamic debug v2 - infrastructure

Jason Baron
2008-08-13T19:29:21Z

On Tue, Aug 12, 2008 at 06:08:04PM -0700, Greg KH wrote:
> On Tue, Aug 12, 2008 at 04:46:19PM -0400, Jason Baron wrote:
> > On Tue, Aug 12, 2008 at 01:09:08PM -0700, Greg KH wrote:
> > > So close, can I have a good changelog comment with the patch so people
> > > know what it is when they look in the logs?
> > >
> > > Care to resend it with that?
> > >
> >
> > Base infrastructure to enable per-module debug messages.
>
> Ah, so close...
>
> With this patch, I get the following build error:
>
> CC [M] drivers/usb/gadget/u_ether.o
> drivers/usb/gadget/u_ether.c: In function ‘gether_setup’:
> drivers/usb/gadget/u_ether.c:787: error: ‘KBUILD_MODNAME’ undeclared (first use in this function)
> drivers/usb/gadget/u_ether.c:787: error: (Each undeclared identifier is reported only once
> drivers/usb/gadget/u_ether.c:787: error: for each function it appears in.)
> drivers/usb/gadget/u_ether.c:787: error: unknown field ‘Usage’ specified in initializer
> drivers/usb/gadget/u_ether.c:787: error: expected expression before ‘.’ token
> drivers/usb/gadget/u_ether.c:787: error: initializer element is not constant
> drivers/usb/gadget/u_ether.c:787: error: (near initialization for ‘descriptor.hash’)
> drivers/usb/gadget/u_ether.c:787: error: ‘Usage’ undeclared (first use in this function)
> drivers/usb/gadget/u_ether.c:787: error: invalid operands to binary << (have ‘long long int’ and ‘char *’)
> drivers/usb/gadget/u_ether.c:787: error: expected ‘)’ before ‘:’ token
> drivers/usb/gadget/u_ether.c:787: error: invalid operands to binary & (have ‘long long int’ and ‘char *’)
> drivers/usb/gadget/u_ether.c:787: error: invalid operands to binary << (have ‘long long int’ and ‘char *’)
> drivers/usb/gadget/u_ether.c:787: error: expected ‘)’ before ‘:’ token
> drivers/usb/gadget/u_ether.c:787: error: invalid operands to binary & (have ‘long long int’ and ‘char *’)
> drivers/usb/gadget/u_ether.c:787: error: expected ‘)’ before ‘:’ token
> drivers/usb/gadget/u_ether.c:787: warning: passing argument 4 of ‘__dynamic_dbg_enabled_helper’ makes integer from pointer without a cast
> drivers/usb/gadget/u_ether.c:787: error: expected ‘)’ before ‘KBUILD_MODNAME’
> drivers/usb/gadget/u_ether.c:787: warning: too few arguments for format
> make[1]: *** [drivers/usb/gadget/u_ether.o] Error 1
> make: *** [_module_drivers/usb/gadget] Error 2
>
>
> Did you try a simple 'make allmodconfig'?
>
> Also, I cleaned it up a bit to make it pass checkpatch.pl and sparse,
> doesn't anyone run these things anymore...
>
> I've attached my fixed up version below.
>
> Any ideas?
>

I was working on an older kernel and so i didn't see this error.

There seem to be a couple of issues here.

First, KBUILD_MODNAME is not defined during the compile of u_ether.c because
it is included from multiple sources. It seems that when there are multiple
sources "modname" is set to a string of all the source names with spaces in
between. So something used in foo and bar looks like: "foo bar". From
scripts/Makefile.lib:

# Note: It's possible that one object gets potentially linked into more
# than one module. In that case KBUILD_MODNAME will be set to foo_bar,
# where foo and bar are the name of the modules.
name-fix = $(subst $(comma),_,$(subst -,_,$1))
basename_flags = -D"KBUILD_BASENAME=KBUILD_STR($(call name-fix,$(basetarget)))"
modname_flags = $(if $(filter 1,$(words $(modname))),\
-D"KBUILD_MODNAME=KBUILD_STR($(call name-fix,$(modname)))")
.
.
.

So, the comment says that the KBUILD_MODNAME should be set to "foo_bar" if
its included from multiple files...however, the modname_flags definition is not
doing that. In fact, if there are multiple sources its simply setting a
NULL modname_flag. This issue is independent of the patchset I posted, and
probably is best fixed separately. A patch which resolves this issue:

Signed-off-by: Jason Baron <jbaron@redhat.com>

---

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index b4ca38a..639d5dc 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -94,7 +94,8 @@ obj-dirs := $(addprefix $(obj)/,$(obj-dirs))
name-fix = $(subst $(comma),_,$(subst -,_,$1))
basename_flags = -D"KBUILD_BASENAME=KBUILD_STR($(call name-fix,$(basetarget)))"
modname_flags = $(if $(filter 1,$(words $(modname))),\
- -D"KBUILD_MODNAME=KBUILD_STR($(call name-fix,$(modname)))")
+ -D"KBUILD_MODNAME=KBUILD_STR($(call name-fix,$(modname)))",\
+ -D"KBUILD_MODNAME=KBUILD_STR($(subst $(space),_,$(call name-fix,$(modname))))")

#hash values
ifdef CONFIG_DYNAMIC_PRINTK_DEBUG

The second issue is then that the hash functions that i introduced don't like
these spaces either. So on top of the patch set i posted please add the patch
below. The kernel should then compile. I tested this on the 'linux-next' tree,
which did not boot on the system i was using, but i'm not sure it was related
to this change.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@redhat.com>

---

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 639d5dc..3b5455b 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -99,8 +99,8 @@ modname_flags = $(if $(filter 1,$(words $(modname))),\

#hash values
ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
-debug_flags = -D"DEBUG_HASH=$(shell ./scripts/basic/hash djb2 $(@D)$(modname))"\
- -D"DEBUG_HASH2=$(shell ./scripts/basic/hash r5 $(@D)$(modname))"
+debug_flags = -D"DEBUG_HASH=$(shell ./scripts/basic/hash djb2 $(@D)$(subst $(space),_,$(modname)))"\
+ -D"DEBUG_HASH2=$(shell ./scripts/basic/hash r5 $(@D)$(subst $(space),_,$(modname)))"
else
debug_flags =
endif