Re: linux-next: driver-core tree build failure

Re: linux-next: driver-core tree build failure

[Geert Uytterhoeven]

On Tue, 10 Mar 2009, Stephen Rothwell wrote:
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> crypto/zlib.c: In function 'zlib_compress_update':
> crypto/zlib.c:148: warning: initialization makes integer from pointer without a cast
> crypto/zlib.c:148: error: initializer element is not computable at load time
> crypto/zlib.c:148: error: (near initialization for 'descriptor.primary_hash')
> crypto/zlib.c:148: warning: excess elements in struct initializer
> crypto/zlib.c:148: warning: (near initialization for 'descriptor')
> 
> And many more similar.  This line is a pr_debug() statement, so the
> finger points at commit 25b67b75587d43ff3f09ad88c03c70a38372d95d
> ("dynamic debug: combine dprintk and dynamic printk") from the
> driver-core tree.
> 
> The preprocessed code looks like this:
> 
> static int zlib_compress_update(struct crypto_pcomp *tfm,
>     struct comp_request *req)
> {
>  int ret;
>  struct zlib_ctx *dctx = crypto_tfm_ctx(crypto_pcomp_tfm(tfm));
>  struct z_stream_s *stream = &dctx->comp_stream;
> 
>  do { do { static struct _ddebug descriptor __attribute__((__used__)) __attribute__((section("__verbose"), aligned(8))) = { "zlib", __func__, "/scratch/sfr/next/crypto/zlib.c", "%s: " "avail_in %u, avail_out %u\n", __func__, 55, 33, 148, 0 }; if (({ int __ret = 0; if (__builtin_expect(!!((dynamic_debug_enabled & (1LL << 55)) && (dynamic_debug_enabled2 & (1LL << 33))), 0)) if (__builtin_expect(!!(descriptor.flags), 0)) __ret = 1; __ret; })) printk("<7>" "zlib" ":" "%s: " "avail_in %u, avail_out %u\n", __func__, req->avail_in, req->avail_out); } while (0); } while (0);
> 
> The problem is the line:
> 
> #define pr_fmt(fmt)     "%s: " fmt, __func__
> 
> in crypto/zlib.c which was introduced by commit
> bf68e65ec9ea61e32ab71bef59aa5d24d255241f ("crypto: zlib - New zlib crypto
> module, using pcomp") from the crypto tree.
> 
> For today, I have removed the above line from crypto/zlib.c, but
> something better needs to be done for tomorrow!

I had a closer look. It happens on all archs, if CONFIG_DYNAMIC_DEBUG=y.

crypto/zlib.c has:

    #define pr_fmt(fmt)     "%s: " fmt, __func__

If CONFIG_DYNAMIC_DEBUG is set, include/linux/kernel.h has:

#define pr_debug(fmt, ...) do { \
	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
	} while (0)

include/linux/dynamic_debug.h has:

#define dynamic_pr_debug(fmt, ...) do {					\
	static struct _ddebug descriptor				\
	__used								\
	__attribute__((section("__verbose"), aligned(8))) =		\
	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
	if (__dynamic_dbg_enabled(descriptor))				\
		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
				##__VA_ARGS__);				\
	} while (0)


So in crypto/zlib.c,

| pr_debug("avail_in %u, avail_out %u\n", req->avail_in, req->avail_out);

is expanded to

| do {
|   do {
|     static struct _ddebug descriptor
|       __attribute__((__used__))
|       __attribute__((section("__verbose"), aligned(8))) = {
|       "zlib",
|       __func__,
|       "crypto/zlib.c",
|       "%s: " "avail_in %u, avail_out %u\n",
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	This is the actual format string

|       __func__,
        ^^^^^^^^^
	But this is the first parameter, which should not be here!!!

|       55,
|       33,
|       150,
|       0
|     };
|     if (({
|       int __ret = 0;
|       if (__builtin_expect(!!((dynamic_debug_enabled & (1LL << 55)) &&
| 			      (dynamic_debug_enabled2 & (1LL << 33))), 0))
| 	if (__builtin_expect(!!(descriptor.flags), 0))
| 	    __ret = 1;
|       __ret;
|     }))
|     printk("<7>" "zlib" ":" "%s: " "avail_in %u, avail_out %u\n", __func__,
| 	   req->avail_in, req->avail_out);
|   } while (0);
| } while (0);

Due the the superfluous `__func__', all field members are shifted by one
position, and compilation breaks.

Apparently inside dynamic_pr_debug(), `fmt' is:

    "avail_in %u, avail_out %u\n", __func__
    
instead of only the part before the comma:

    "avail_in %u, avail_out %u\n"


For the non-CONFIG_DYNAMIC_DEBUG case, pr_debug() is expanded correctly:

DEBUG is defined:

#define pr_debug(fmt, ...) \
	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)

and the line is expanded to:

| printk("<7>" "%s: " "avail_in %u, avail_out %u\n", __func__, req->avail_in, req->avail_out);

DEBUG is not defined:

#define pr_debug(fmt, ...) \
	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })

and the line is expanded to:

| ({ if (0) printk("<7>" "%s: " "avail_in %u, avail_out %u\n", __func__, req->avail_in, req->avail_out); 0; });

Why doesn't it work for dynamic_pr_debug()?

BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
intended usage of your pr_fmt() infrastructure?

Thanks!

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: linux-next: driver-core tree build failure

[Herbert Xu]

On Tue, Mar 10, 2009 at 02:31:17PM +0100, Geert Uytterhoeven wrote:
>
> Why doesn't it work for dynamic_pr_debug()?

Because fmt is expanded twice?

> BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> intended usage of your pr_fmt() infrastructure?

I never liked this in the first place :) If anyone wants to debug
this they can add whatever printks they want.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: driver-core tree build failure

[Martin Schwidefsky]

On Tue, 10 Mar 2009 14:31:17 +0100 (CET)
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> crypto/zlib.c has:
> 
>     #define pr_fmt(fmt)     "%s: " fmt, __func__
> 
> If CONFIG_DYNAMIC_DEBUG is set, include/linux/kernel.h has:
> 
> #define pr_debug(fmt, ...) do { \
> 	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> 	} while (0)
> 
> include/linux/dynamic_debug.h has:
> 
> #define dynamic_pr_debug(fmt, ...) do {					\
> 	static struct _ddebug descriptor				\
> 	__used								\
> 	__attribute__((section("__verbose"), aligned(8))) =		\
> 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
> 		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
> 	if (__dynamic_dbg_enabled(descriptor))				\
> 		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
> 				##__VA_ARGS__);				\
> 	} while (0)

The dynamic_pr_debug macro currently works only with pr_fmt definitions
that do not add additional parameters. The way how we use the pr_fmt
macro is:

#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt

The same could be done with the problematic pr_fmt definition:

#define pr_fmt(fmt)     __func__ ": " fmt

> BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> intended usage of your pr_fmt() infrastructure?

The indended use is a simple prefix to the format string. To paste an
additional parameter is an interesting use of the pr_fmt macro ..

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: linux-next: driver-core tree build failure

[Geert Uytterhoeven]

On Tue, 10 Mar 2009, Martin Schwidefsky wrote:
> On Tue, 10 Mar 2009 14:31:17 +0100 (CET)
> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > crypto/zlib.c has:
> > 
> >     #define pr_fmt(fmt)     "%s: " fmt, __func__
> > 
> > If CONFIG_DYNAMIC_DEBUG is set, include/linux/kernel.h has:
> > 
> > #define pr_debug(fmt, ...) do { \
> > 	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> > 	} while (0)
> > 
> > include/linux/dynamic_debug.h has:
> > 
> > #define dynamic_pr_debug(fmt, ...) do {					\
> > 	static struct _ddebug descriptor				\
> > 	__used								\
> > 	__attribute__((section("__verbose"), aligned(8))) =		\
> > 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
> > 		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
> > 	if (__dynamic_dbg_enabled(descriptor))				\
> > 		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
> > 				##__VA_ARGS__);				\
> > 	} while (0)
> 
> The dynamic_pr_debug macro currently works only with pr_fmt definitions
> that do not add additional parameters. The way how we use the pr_fmt
> macro is:
> 
> #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
> 
> The same could be done with the problematic pr_fmt definition:
> 
> #define pr_fmt(fmt)     __func__ ": " fmt

No, that also doesn't work:

| crypto/zlib.c:150: error: expected '}' before string constant
| crypto/zlib.c:150: error: expected ')' before '__func__'
| crypto/zlib.c:162: error: expected '}' before string constant
| crypto/zlib.c:162: error: expected ')' before '__func__'
| crypto/zlib.c:166: error: expected '}' before string constant
| crypto/zlib.c:166: error: expected ')' before '__func__'
| crypto/zlib.c:170: error: expected '}' before string constant
| crypto/zlib.c:170: error: expected ')' before '__func__'

> > BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> > intended usage of your pr_fmt() infrastructure?
> 
> The indended use is a simple prefix to the format string. To paste an
> additional parameter is an interesting use of the pr_fmt macro ..

Bummer, I was so happy I could do things like

| #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__

...

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: linux-next: driver-core tree build failure

[Martin Schwidefsky]

On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> > The same could be done with the problematic pr_fmt definition:
> > 
> > #define pr_fmt(fmt)     __func__ ": " fmt
> 
> No, that also doesn't work:
> 
> | crypto/zlib.c:150: error: expected '}' before string constant
> | crypto/zlib.c:150: error: expected ')' before '__func__'
> | crypto/zlib.c:162: error: expected '}' before string constant
> | crypto/zlib.c:162: error: expected ')' before '__func__'
> | crypto/zlib.c:166: error: expected '}' before string constant
> | crypto/zlib.c:166: error: expected ')' before '__func__'
> | crypto/zlib.c:170: error: expected '}' before string constant
> | crypto/zlib.c:170: error: expected ')' before '__func__'
> 
> > > BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> > > intended usage of your pr_fmt() infrastructure?
> > 
> > The indended use is a simple prefix to the format string. To paste an
> > additional parameter is an interesting use of the pr_fmt macro ..
> 
> Bummer, I was so happy I could do things like
> 
> | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__

Actually that seem like a nice thing to have. With the upstream version of
dynamic_pr_debug this works, there the format string is used only on a printk.
git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
that pastes the format string to the _ddebug structure.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: linux-next: driver-core tree build failure

[Jason Baron]

On Tue, Mar 10, 2009 at 05:08:41PM +0100, Martin Schwidefsky wrote:
> On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > > The same could be done with the problematic pr_fmt definition:
> > > 
> > > #define pr_fmt(fmt)     __func__ ": " fmt
> > 
> > No, that also doesn't work:
> > 
> > | crypto/zlib.c:150: error: expected '}' before string constant
> > | crypto/zlib.c:150: error: expected ')' before '__func__'
> > | crypto/zlib.c:162: error: expected '}' before string constant
> > | crypto/zlib.c:162: error: expected ')' before '__func__'
> > | crypto/zlib.c:166: error: expected '}' before string constant
> > | crypto/zlib.c:166: error: expected ')' before '__func__'
> > | crypto/zlib.c:170: error: expected '}' before string constant
> > | crypto/zlib.c:170: error: expected ')' before '__func__'
> > 
> > > > BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> > > > intended usage of your pr_fmt() infrastructure?
> > > 
> > > The indended use is a simple prefix to the format string. To paste an
> > > additional parameter is an interesting use of the pr_fmt macro ..
> > 
> > Bummer, I was so happy I could do things like
> > 
> > | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__
> 
> Actually that seem like a nice thing to have. With the upstream version of
> dynamic_pr_debug this works, there the format string is used only on a printk.
> git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
> that pastes the format string to the _ddebug structure.
> 

hmmm...yeah, some macro magic in include/linux/dynamic_debug.h converts
the 'fmt' arg into a series of strings. It doesn't look as pretty in the
dynamic debug control file:

crypto/zlib.c:333 [zlib]zlib_decompress_final - "\042%s: \042
\042avail_in %u, avail_out %u (consumed %u, produced %u)\n\042,
__func__"

with all those '\042' there, which are the '"' characters, but we
probably could live with it.

patch below.

thanks,

-Jason



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


diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 07781aa..16cf212 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -50,28 +50,30 @@ extern int ddebug_remove_module(char *mod_name);
 					__ret = 1;			     \
 	__ret; })
 
-#define dynamic_pr_debug(fmt, ...) do {					\
-	static struct _ddebug descriptor				\
-	__used								\
-	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
-		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
-	if (__dynamic_dbg_enabled(descriptor))				\
-		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
-				##__VA_ARGS__);				\
+#define STRINGIFY(args...) #args
+
+#define dynamic_pr_debug(fmt, ...) do {					  \
+	static struct _ddebug descriptor				  \
+	__used								  \
+	__attribute__((section("__verbose"), aligned(8))) =		  \
+	{ KBUILD_MODNAME, __func__, __FILE__, STRINGIFY(fmt), DEBUG_HASH, \
+		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	  \
+	if (__dynamic_dbg_enabled(descriptor))				  \
+		printk(KERN_DEBUG KBUILD_MODNAME ":" 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, DEBUG_HASH,	\
-		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
-	if (__dynamic_dbg_enabled(descriptor))				\
-			dev_printk(KERN_DEBUG, dev,			\
-					KBUILD_MODNAME ": " fmt,	\
-					##__VA_ARGS__);			\
+#define dynamic_dev_dbg(dev, fmt, ...) do {				  \
+	static struct _ddebug descriptor				  \
+	__used								  \
+	__attribute__((section("__verbose"), aligned(8))) =		  \
+	{ KBUILD_MODNAME, __func__, __FILE__, STRINGIFY(fmt), DEBUG_HASH, \
+		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	  \
+	if (__dynamic_dbg_enabled(descriptor))				  \
+			dev_printk(KERN_DEBUG, dev,			  \
+					KBUILD_MODNAME ": " fmt,	  \
+					##__VA_ARGS__);			  \
 	} while (0)
 
 #else

Re: linux-next: driver-core tree build failure

[Greg KH]

On Tue, Mar 10, 2009 at 04:02:00PM -0400, Jason Baron wrote:
> On Tue, Mar 10, 2009 at 05:08:41PM +0100, Martin Schwidefsky wrote:
> > On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > 
> > > > The same could be done with the problematic pr_fmt definition:
> > > > 
> > > > #define pr_fmt(fmt)     __func__ ": " fmt
> > > 
> > > No, that also doesn't work:
> > > 
> > > | crypto/zlib.c:150: error: expected '}' before string constant
> > > | crypto/zlib.c:150: error: expected ')' before '__func__'
> > > | crypto/zlib.c:162: error: expected '}' before string constant
> > > | crypto/zlib.c:162: error: expected ')' before '__func__'
> > > | crypto/zlib.c:166: error: expected '}' before string constant
> > > | crypto/zlib.c:166: error: expected ')' before '__func__'
> > > | crypto/zlib.c:170: error: expected '}' before string constant
> > > | crypto/zlib.c:170: error: expected ')' before '__func__'
> > > 
> > > > > BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> > > > > intended usage of your pr_fmt() infrastructure?
> > > > 
> > > > The indended use is a simple prefix to the format string. To paste an
> > > > additional parameter is an interesting use of the pr_fmt macro ..
> > > 
> > > Bummer, I was so happy I could do things like
> > > 
> > > | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__
> > 
> > Actually that seem like a nice thing to have. With the upstream version of
> > dynamic_pr_debug this works, there the format string is used only on a printk.
> > git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
> > that pastes the format string to the _ddebug structure.
> > 
> 
> hmmm...yeah, some macro magic in include/linux/dynamic_debug.h converts
> the 'fmt' arg into a series of strings. It doesn't look as pretty in the
> dynamic debug control file:
> 
> crypto/zlib.c:333 [zlib]zlib_decompress_final - "\042%s: \042
> \042avail_in %u, avail_out %u (consumed %u, produced %u)\n\042,
> __func__"
> 
> with all those '\042' there, which are the '"' characters, but we
> probably could live with it.
> 
> patch below.

Can you resend this with a description of what the patch does so I can
apply it?

thanks,

greg k-h

Re: linux-next: driver-core tree build failure

[Geert Uytterhoeven]

On Tue, 10 Mar 2009, Jason Baron wrote:
> On Tue, Mar 10, 2009 at 05:08:41PM +0100, Martin Schwidefsky wrote:
> > On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > 
> > > > The same could be done with the problematic pr_fmt definition:
> > > > 
> > > > #define pr_fmt(fmt)     __func__ ": " fmt
> > > 
> > > No, that also doesn't work:
> > > 
> > > | crypto/zlib.c:150: error: expected '}' before string constant
> > > | crypto/zlib.c:150: error: expected ')' before '__func__'
> > > | crypto/zlib.c:162: error: expected '}' before string constant
> > > | crypto/zlib.c:162: error: expected ')' before '__func__'
> > > | crypto/zlib.c:166: error: expected '}' before string constant
> > > | crypto/zlib.c:166: error: expected ')' before '__func__'
> > > | crypto/zlib.c:170: error: expected '}' before string constant
> > > | crypto/zlib.c:170: error: expected ')' before '__func__'
> > > 
> > > > > BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> > > > > intended usage of your pr_fmt() infrastructure?
> > > > 
> > > > The indended use is a simple prefix to the format string. To paste an
> > > > additional parameter is an interesting use of the pr_fmt macro ..
> > > 
> > > Bummer, I was so happy I could do things like
> > > 
> > > | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__
> > 
> > Actually that seem like a nice thing to have. With the upstream version of
> > dynamic_pr_debug this works, there the format string is used only on a printk.
> > git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
> > that pastes the format string to the _ddebug structure.
> > 
> 
> hmmm...yeah, some macro magic in include/linux/dynamic_debug.h converts
> the 'fmt' arg into a series of strings. It doesn't look as pretty in the
> dynamic debug control file:
> 
> crypto/zlib.c:333 [zlib]zlib_decompress_final - "\042%s: \042
> \042avail_in %u, avail_out %u (consumed %u, produced %u)\n\042,
> __func__"
> 
> with all those '\042' there, which are the '"' characters, but we
> probably could live with it.

Ugh, not only that, it also contains the __func__.

BTW, why do you store this string? The only thing you can do with it later is
to print it verbatim (without % format parameter expansion), as it has
swallowed the first parameter (__func__), but still contains the %s.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: linux-next: driver-core tree build failure

[Greg Banks]

Jason Baron wrote:
> On Tue, Mar 10, 2009 at 05:08:41PM +0100, Martin Schwidefsky wrote:
>   
>> On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
>> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
>>
>>     
>>>> The same could be done with the problematic pr_fmt definition:
>>>>
>>>> #define pr_fmt(fmt)     __func__ ": " fmt
>>>>         
>>> No, that also doesn't work:
>>>
>>> | crypto/zlib.c:150: error: expected '}' before string constant
>>> | crypto/zlib.c:150: error: expected ')' before '__func__'
>>> | crypto/zlib.c:162: error: expected '}' before string constant
>>> | crypto/zlib.c:162: error: expected ')' before '__func__'
>>> | crypto/zlib.c:166: error: expected '}' before string constant
>>> | crypto/zlib.c:166: error: expected ')' before '__func__'
>>> | crypto/zlib.c:170: error: expected '}' before string constant
>>> | crypto/zlib.c:170: error: expected ')' before '__func__'
>>>
>>>       
>>>>> BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
>>>>> intended usage of your pr_fmt() infrastructure?
>>>>>           
>>>> The indended use is a simple prefix to the format string. To paste an
>>>> additional parameter is an interesting use of the pr_fmt macro ..
>>>>         
>>> Bummer, I was so happy I could do things like
>>>
>>> | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__
>>>       
>> Actually that seem like a nice thing to have. With the upstream version of
>> dynamic_pr_debug this works, there the format string is used only on a printk.
>> git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
>> that pastes the format string to the _ddebug structure.
>>
>>     
>
> hmmm...yeah, some macro magic in include/linux/dynamic_debug.h converts
> the 'fmt' arg into a series of strings. It doesn't look as pretty in the
> dynamic debug control file:
>
> crypto/zlib.c:333 [zlib]zlib_decompress_final - "\042%s: \042
> \042avail_in %u, avail_out %u (consumed %u, produced %u)\n\042,
> __func__"
>
> with all those '\042' there, which are the '"' characters, but we
> probably could live with it.
>
> patch below.
>
>   

I think this patch does the same thing more cleanly.

When CONFIG_DYNAMIC_DEBUG is enabled, allow callers of pr_debug()
to provide their own definition of pr_fmt() even if that definition
uses tricks like

#define pr_fmt(fmt) "%s:" fmt, __func__

Signed-off-by: Greg Banks <gnb@sgi.com>
---

 include/linux/dynamic_debug.h |    4 ++--
 include/linux/kernel.h        |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

Index: linux-git/include/linux/dynamic_debug.h
===================================================================
--- linux-git.orig/include/linux/dynamic_debug.h
+++ linux-git/include/linux/dynamic_debug.h
@@ -57,7 +57,7 @@ extern int ddebug_remove_module(char *mo
 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
 		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
 	if (__dynamic_dbg_enabled(descriptor))				\
-		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
+		printk(KERN_DEBUG KBUILD_MODNAME ":" pr_fmt(fmt),	\
 				##__VA_ARGS__);				\
 	} while (0)
 
@@ -70,7 +70,7 @@ extern int ddebug_remove_module(char *mo
 		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
 	if (__dynamic_dbg_enabled(descriptor))				\
 			dev_printk(KERN_DEBUG, dev,			\
-					KBUILD_MODNAME ": " fmt,	\
+					KBUILD_MODNAME ": " pr_fmt(fmt),\
 					##__VA_ARGS__);			\
 	} while (0)
 
Index: linux-git/include/linux/kernel.h
===================================================================
--- linux-git.orig/include/linux/kernel.h
+++ linux-git/include/linux/kernel.h
@@ -359,8 +359,9 @@ static inline char *pack_hex_byte(char *
 #define pr_debug(fmt, ...) \
 	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #elif defined(CONFIG_DYNAMIC_DEBUG)
+/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
 #define pr_debug(fmt, ...) do { \
-	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
+	dynamic_pr_debug(fmt, ##__VA_ARGS__); \
 	} while (0)
 #else
 #define pr_debug(fmt, ...) \



-- 
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.

Re: linux-next: driver-core tree build failure

[Geert Uytterhoeven]

On Wed, 11 Mar 2009, Greg Banks wrote:
> Jason Baron wrote:
> > On Tue, Mar 10, 2009 at 05:08:41PM +0100, Martin Schwidefsky wrote:
> >   
> >> On Tue, 10 Mar 2009 16:29:50 +0100 (CET)
> >> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> >>
> >>     
> >>>> The same could be done with the problematic pr_fmt definition:
> >>>>
> >>>> #define pr_fmt(fmt)     __func__ ": " fmt
> >>>>         
> >>> No, that also doesn't work:
> >>>
> >>> | crypto/zlib.c:150: error: expected '}' before string constant
> >>> | crypto/zlib.c:150: error: expected ')' before '__func__'
> >>> | crypto/zlib.c:162: error: expected '}' before string constant
> >>> | crypto/zlib.c:162: error: expected ')' before '__func__'
> >>> | crypto/zlib.c:166: error: expected '}' before string constant
> >>> | crypto/zlib.c:166: error: expected ')' before '__func__'
> >>> | crypto/zlib.c:170: error: expected '}' before string constant
> >>> | crypto/zlib.c:170: error: expected ')' before '__func__'
> >>>
> >>>       
> >>>>> BTW, Martin: Is `#define pr_fmt(fmt)     "%s: " fmt, __func__' a valid and
> >>>>> intended usage of your pr_fmt() infrastructure?
> >>>>>           
> >>>> The indended use is a simple prefix to the format string. To paste an
> >>>> additional parameter is an interesting use of the pr_fmt macro ..
> >>>>         
> >>> Bummer, I was so happy I could do things like
> >>>
> >>> | #define pr_fmt(fmt)	"%s:%u: " fmt, __func__, __LINE__
> >>>       
> >> Actually that seem like a nice thing to have. With the upstream version of
> >> dynamic_pr_debug this works, there the format string is used only on a printk.
> >> git commit 25b67b75587d43ff3f09ad88c03c70a38372d95d introduces the code
> >> that pastes the format string to the _ddebug structure.
> >>
> >>     
> >
> > hmmm...yeah, some macro magic in include/linux/dynamic_debug.h converts
> > the 'fmt' arg into a series of strings. It doesn't look as pretty in the
> > dynamic debug control file:
> >
> > crypto/zlib.c:333 [zlib]zlib_decompress_final - "\042%s: \042
> > \042avail_in %u, avail_out %u (consumed %u, produced %u)\n\042,
> > __func__"
> >
> > with all those '\042' there, which are the '"' characters, but we
> > probably could live with it.
> >
> > patch below.
> >
> >   
> 
> I think this patch does the same thing more cleanly.
> 
> When CONFIG_DYNAMIC_DEBUG is enabled, allow callers of pr_debug()
> to provide their own definition of pr_fmt() even if that definition
> uses tricks like
> 
> #define pr_fmt(fmt) "%s:" fmt, __func__
> 
> Signed-off-by: Greg Banks <gnb@sgi.com>

Thanks, much cleaner!

Acked-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

> ---
> 
>  include/linux/dynamic_debug.h |    4 ++--
>  include/linux/kernel.h        |    3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> Index: linux-git/include/linux/dynamic_debug.h
> ===================================================================
> --- linux-git.orig/include/linux/dynamic_debug.h
> +++ linux-git/include/linux/dynamic_debug.h
> @@ -57,7 +57,7 @@ extern int ddebug_remove_module(char *mo
>  	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
>  		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
>  	if (__dynamic_dbg_enabled(descriptor))				\
> -		printk(KERN_DEBUG KBUILD_MODNAME ":" fmt,		\
> +		printk(KERN_DEBUG KBUILD_MODNAME ":" pr_fmt(fmt),	\
>  				##__VA_ARGS__);				\
>  	} while (0)
>  
> @@ -70,7 +70,7 @@ extern int ddebug_remove_module(char *mo
>  		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
>  	if (__dynamic_dbg_enabled(descriptor))				\
>  			dev_printk(KERN_DEBUG, dev,			\
> -					KBUILD_MODNAME ": " fmt,	\
> +					KBUILD_MODNAME ": " pr_fmt(fmt),\
>  					##__VA_ARGS__);			\
>  	} while (0)
>  
> Index: linux-git/include/linux/kernel.h
> ===================================================================
> --- linux-git.orig/include/linux/kernel.h
> +++ linux-git/include/linux/kernel.h
> @@ -359,8 +359,9 @@ static inline char *pack_hex_byte(char *
>  #define pr_debug(fmt, ...) \
>  	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>  #elif defined(CONFIG_DYNAMIC_DEBUG)
> +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
>  #define pr_debug(fmt, ...) do { \
> -	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> +	dynamic_pr_debug(fmt, ##__VA_ARGS__); \
>  	} while (0)
>  #else
>  #define pr_debug(fmt, ...) \

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: linux-next: driver-core tree build failure

[Jason Baron]

On Wed, Mar 11, 2009 at 09:07:28PM +1100, Greg Banks wrote:
> 
> I think this patch does the same thing more cleanly.
> 
> When CONFIG_DYNAMIC_DEBUG is enabled, allow callers of pr_debug()
> to provide their own definition of pr_fmt() even if that definition
> uses tricks like
> 
> #define pr_fmt(fmt) "%s:" fmt, __func__
> 

patch looks good. I agree its simpler than what I proposed. However, I
don't think we want to add pr_fmt() in the dynamic_dev_dbg() path, since
dev_dbg() isn't doing that to start with. Other than that, I ack it.

thanks,

-Jason

linux-next: driver-core tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

Hi Greg,

Today's linux-next build (arm omap_osk_5912_defconfig) failed like this:

drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_num':
drivers/video/omap/omapfb_main.c:1253: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_text':
drivers/video/omap/omapfb_main.c:1273: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_show_panel_name':
drivers/video/omap/omapfb_main.c:1320: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_level':
drivers/video/omap/omapfb_main.c:1329: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_store_bklight_level':
drivers/video/omap/omapfb_main.c:1344: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_max':
drivers/video/omap/omapfb_main.c:1363: error: 'struct device' has no member named 'driver_data'
drivers/video/omap/omapfb_main.c: In function 'omapfb_show_ctrl_name':
drivers/video/omap/omapfb_main.c:1396: error: 'struct device' has no member named 'driver_data'

Caused by commit 53d8ba57f16029a47ed5df9840ab6c3f9abd3727 ("Driver core:
move dev_get/set_drvdata to drivers/base/dd.c") from the driver-core tree.

I see more grepping in your future :-)
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: driver-core tree build failure

[Greg KH]

On Tue, Jun 23, 2009 at 04:01:00PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next build (arm omap_osk_5912_defconfig) failed like this:
> 
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_num':
> drivers/video/omap/omapfb_main.c:1253: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_text':
> drivers/video/omap/omapfb_main.c:1273: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_panel_name':
> drivers/video/omap/omapfb_main.c:1320: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_level':
> drivers/video/omap/omapfb_main.c:1329: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_store_bklight_level':
> drivers/video/omap/omapfb_main.c:1344: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_max':
> drivers/video/omap/omapfb_main.c:1363: error: 'struct device' has no member named 'driver_data'
> drivers/video/omap/omapfb_main.c: In function 'omapfb_show_ctrl_name':
> drivers/video/omap/omapfb_main.c:1396: error: 'struct device' has no member named 'driver_data'
> 
> Caused by commit 53d8ba57f16029a47ed5df9840ab6c3f9abd3727 ("Driver core:
> move dev_get/set_drvdata to drivers/base/dd.c") from the driver-core tree.
> 
> I see more grepping in your future :-)

Heh, thanks, I got i386, x86-64, and ppc64 working properly this time
around, but new things keep slipping in :)

I'll go resolve these as well.

thanks,

greg k-h

Re: linux-next: driver-core tree build failure

[Greg KH]

On Tue, Jun 23, 2009 at 08:29:16AM -0700, Greg KH wrote:
> On Tue, Jun 23, 2009 at 04:01:00PM +1000, Stephen Rothwell wrote:
> > Hi Greg,
> > 
> > Today's linux-next build (arm omap_osk_5912_defconfig) failed like this:
> > 
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_num':
> > drivers/video/omap/omapfb_main.c:1253: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_caps_text':
> > drivers/video/omap/omapfb_main.c:1273: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_panel_name':
> > drivers/video/omap/omapfb_main.c:1320: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_level':
> > drivers/video/omap/omapfb_main.c:1329: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_store_bklight_level':
> > drivers/video/omap/omapfb_main.c:1344: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_bklight_max':
> > drivers/video/omap/omapfb_main.c:1363: error: 'struct device' has no member named 'driver_data'
> > drivers/video/omap/omapfb_main.c: In function 'omapfb_show_ctrl_name':
> > drivers/video/omap/omapfb_main.c:1396: error: 'struct device' has no member named 'driver_data'
> > 
> > Caused by commit 53d8ba57f16029a47ed5df9840ab6c3f9abd3727 ("Driver core:
> > move dev_get/set_drvdata to drivers/base/dd.c") from the driver-core tree.
> > 
> > I see more grepping in your future :-)
> 
> Heh, thanks, I got i386, x86-64, and ppc64 working properly this time
> around, but new things keep slipping in :)
> 
> I'll go resolve these as well.

Now fixed and pushed out.

thanks,

greg k-h

linux-next: driver-core tree build failure

[Stephen Rothwell]

Hi Greg,

Today's linux-next build (x86_64 allmodconfig) failed like this:

drivers/mtd/mtdcore.c:220: error: expected '{' before 'const'
drivers/mtd/mtdcore.c:227: error: 'mtd_groups' undeclared here (not in a function)

Caused by the update to commit ba87b739a1f5dd545679d886b8d193cdede991cf
("driver model: constify attribute groups").

I have applied the patch below for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 14 Jul 2009 16:29:39 +1000
Subject: [PATCH] driver-core: fix for mtd typo

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/mtd/mtdcore.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 93e6731..05a16be 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -217,7 +217,7 @@ struct attribute_group mtd_group = {
 	.attrs		= mtd_attrs,
 };
 
-struct const attribute_group *mtd_groups[] = {
+const struct attribute_group *mtd_groups[] = {
 	&mtd_group,
 	NULL,
 };
-- 
1.6.3.3

Re: linux-next: driver-core tree build failure

[David Brownell]

On Monday 13 July 2009, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> drivers/mtd/mtdcore.c:220: error: expected '{' before 'const'
> drivers/mtd/mtdcore.c:227: error: 'mtd_groups' undeclared here (not in a function)
> 
> Caused by the update to commit ba87b739a1f5dd545679d886b8d193cdede991cf
> ("driver model: constify attribute groups").
> 
> I have applied the patch below for today.

That's a good patch, go with it.  Sorry; I guess this wasn't
the version I had test-built.  My eyes were reporting "static"
as the (expected) initial keyword there; they lied!


> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 14 Jul 2009 16:29:39 +1000
> Subject: [PATCH] driver-core: fix for mtd typo
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/mtd/mtdcore.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 93e6731..05a16be 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -217,7 +217,7 @@ struct attribute_group mtd_group = {
>  	.attrs		= mtd_attrs,
>  };
>  
> -struct const attribute_group *mtd_groups[] = {
> +const struct attribute_group *mtd_groups[] = {
>  	&mtd_group,
>  	NULL,
>  };
> -- 
> 1.6.3.3
> 
> 

Re: linux-next: driver-core tree build failure

[Greg KH]

On Tue, Jul 14, 2009 at 12:31:27AM -0700, David Brownell wrote:
> On Monday 13 July 2009, Stephen Rothwell wrote:
> > Hi Greg,
> > 
> > Today's linux-next build (x86_64 allmodconfig) failed like this:
> > 
> > drivers/mtd/mtdcore.c:220: error: expected '{' before 'const'
> > drivers/mtd/mtdcore.c:227: error: 'mtd_groups' undeclared here (not in a function)
> > 
> > Caused by the update to commit ba87b739a1f5dd545679d886b8d193cdede991cf
> > ("driver model: constify attribute groups").
> > 
> > I have applied the patch below for today.
> 
> That's a good patch, go with it.  Sorry; I guess this wasn't
> the version I had test-built.  My eyes were reporting "static"
> as the (expected) initial keyword there; they lied!

I've merged this with your original one, thanks.

greg k-h