[PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Wu Zhangjin]

This patch will fix the following warning:

kernel/time/tick-sched.c: In function 'tick_nohz_stop_sched_tick':
kernel/time/tick-sched.c:261: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'long unsigned int'

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 kernel/time/tick-sched.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index e0f59a2..26370b0 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -257,7 +257,7 @@ void tick_nohz_stop_sched_tick(int inidle)
 		static int ratelimit;
 
 		if (ratelimit < 10) {
-			printk(KERN_ERR "NOHZ: local_softirq_pending %02x\n",
+			printk(KERN_ERR "NOHZ: local_softirq_pending %02lx\n",
 			       local_softirq_pending());
 			ratelimit++;
 		}
-- 
1.6.2.1

Re: [PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Linus Torvalds]

On Thu, 8 Oct 2009, Wu Zhangjin wrote:
>
> This patch will fix the following warning:

No it won't. It will add a lot of new warnings.

The thing is, almost all architectures (including x86) have

	unsigned int __softirq_pending;

but then in <asm-generic/hardirq.h> we have

	unsigned long __softirq_pending;

for some unfathomable reason. Quite frankly, I think Arnd just screwed up 
the "generic" version, and the fix is almost certainly to just make the 
generic version match all the main architectures. 

I don't have any architectures using the generic header file, though, so 
I'm not going to do that change blindly. People who do should look at it 
(alpha, powerpc and mips look like the only ones that might be 64-bit, but 
I didn't check very carefully - just grepped for it)

Added Cc's for some people that have worked on, or used, that generic 
header file. Is there any possible reason why it is "unsigned long" in 
that one?

		Linus

---
> kernel/time/tick-sched.c: In function 'tick_nohz_stop_sched_tick':
> kernel/time/tick-sched.c:261: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'long unsigned int'
> 
> Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> ---
>  kernel/time/tick-sched.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index e0f59a2..26370b0 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -257,7 +257,7 @@ void tick_nohz_stop_sched_tick(int inidle)
>  		static int ratelimit;
>  
>  		if (ratelimit < 10) {
> -			printk(KERN_ERR "NOHZ: local_softirq_pending %02x\n",
> +			printk(KERN_ERR "NOHZ: local_softirq_pending %02lx\n",
>  			       local_softirq_pending());
>  			ratelimit++;
>  		}
> -- 
> 1.6.2.1
> 

Re: [PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Wu Zhangjin]

On Thu, 2009-10-08 at 08:03 -0700, Linus Torvalds wrote:
> 
> On Thu, 8 Oct 2009, Wu Zhangjin wrote:
> >
> > This patch will fix the following warning:
> 
> No it won't. It will add a lot of new warnings.
> 
> The thing is, almost all architectures (including x86) have
> 
> 	unsigned int __softirq_pending;
> 
> but then in <asm-generic/hardirq.h> we have
> 
> 	unsigned long __softirq_pending;
> 
> for some unfathomable reason. Quite frankly, I think Arnd just screwed up 
> the "generic" version, and the fix is almost certainly to just make the 
> generic version match all the main architectures. 
> 
> I don't have any architectures using the generic header file, though, so 
> I'm not going to do that change blindly. People who do should look at it 
> (alpha, powerpc and mips look like the only ones that might be 64-bit, but 
> I didn't check very carefully - just grepped for it)
> 
> Added Cc's for some people that have worked on, or used, that generic 
> header file. Is there any possible reason why it is "unsigned long" in 
> that one?
> 

I'm really using a MIPS machine! there is only a "unsigned long"
definition in include/asm-generic/hardirq.h.

Regards,
	Wu Zhangjin

Re: [PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Yong Zhang]

On Thu, Oct 8, 2009 at 11:31 PM, Wu Zhangjin <wuzhangjin@gmail.com> wrote:
> On Thu, 2009-10-08 at 08:03 -0700, Linus Torvalds wrote:
>>
>> On Thu, 8 Oct 2009, Wu Zhangjin wrote:
>> >
>> > This patch will fix the following warning:
>>
>> No it won't. It will add a lot of new warnings.
>>
>> The thing is, almost all architectures (including x86) have
>>
>>       unsigned int __softirq_pending;
>>
>> but then in <asm-generic/hardirq.h> we have
>>
>>       unsigned long __softirq_pending;
>>
>> for some unfathomable reason. Quite frankly, I think Arnd just screwed up
>> the "generic" version, and the fix is almost certainly to just make the
>> generic version match all the main architectures.
>>
>> I don't have any architectures using the generic header file, though, so
>> I'm not going to do that change blindly. People who do should look at it
>> (alpha, powerpc and mips look like the only ones that might be 64-bit, but
>> I didn't check very carefully - just grepped for it)
>>
>> Added Cc's for some people that have worked on, or used, that generic
>> header file. Is there any possible reason why it is "unsigned long" in
>> that one?
>>
>
> I'm really using a MIPS machine! there is only a "unsigned long"
> definition in include/asm-generic/hardirq.h.
>

So this is introduced by 24ffce18a4b6b5e9769200582c09df7ff044259f.
     MIPS: Convert to asm-generic/hardirq.h

Cced Ralf.

Thanks,
Yong

> Regards,
>        Wu Zhangjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Ralf Baechle]

On Thu, Oct 08, 2009 at 08:03:40AM -0700, Linus Torvalds wrote:

> > This patch will fix the following warning:
> 
> No it won't. It will add a lot of new warnings.
> 
> The thing is, almost all architectures (including x86) have
> 
> 	unsigned int __softirq_pending;
> 
> but then in <asm-generic/hardirq.h> we have
> 
> 	unsigned long __softirq_pending;
> 
> for some unfathomable reason. Quite frankly, I think Arnd just screwed up 
> the "generic" version, and the fix is almost certainly to just make the 
> generic version match all the main architectures. 
> 
> I don't have any architectures using the generic header file, though, so 
> I'm not going to do that change blindly. People who do should look at it 
> (alpha, powerpc and mips look like the only ones that might be 64-bit, but 
> I didn't check very carefully - just grepped for it)
> 
> Added Cc's for some people that have worked on, or used, that generic 
> header file. Is there any possible reason why it is "unsigned long" in 
> that one?

I think for no other reason than many other bitfields in kernel being
unsigned long or arrays of unsigned long.  Except of course that
__softirq_pending is never being used with the <asm/bitops.h> operations
that operate on unsigned longs.

Patch in followup email.

  Ralf

[PATCH] IRQ: Change __softirq_pending to unsigned int in asm-generic/hardirq.h.

[Ralf Baechle]

Since the beginnings in aafe4dbed0bf6cbdb2e9f03e1d42f8a540d8541d
include/asm-generic/hardirq.h defined __softirq_pending as unsigned long
which is different from other architectures for no apparent good reason
and was causing the following warning:

kernel/time/tick-sched.c: In function 'tick_nohz_stop_sched_tick':
kernel/time/tick-sched.c:261: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'long unsigned int'

Reported and initial patch by Wu Zhangjin <wuzhangjin@gmail.com>.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
---
Tested on a big endian 64-bit MIPS SMP system.

 include/asm-generic/hardirq.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/hardirq.h b/include/asm-generic/hardirq.h
index 23bb4da..62f5908 100644
--- a/include/asm-generic/hardirq.h
+++ b/include/asm-generic/hardirq.h
@@ -6,7 +6,7 @@
 #include <linux/irq.h>
 
 typedef struct {
-	unsigned long __softirq_pending;
+	unsigned int __softirq_pending;
 } ____cacheline_aligned irq_cpustat_t;
 
 #include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */

Re: [PATCH] kernel/time/tick-sched.c: fix warning of printk's argument format

[Helge Deller]

On 10/09/2009 03:42 PM, Arnd Bergmann wrote:
> On Thursday 08 October 2009, Linus Torvalds wrote:
>> for some unfathomable reason. Quite frankly, I think Arnd just screwed up
>> the "generic" version, and the fix is almost certainly to just make the
>> generic version match all the main architectures.
>>
>> I don't have any architectures using the generic header file, though, so
>> I'm not going to do that change blindly. People who do should look at it
>> (alpha, powerpc and mips look like the only ones that might be 64-bit, but
>> I didn't check very carefully - just grepped for it)
>>
>> Added Cc's for some people that have worked on, or used, that generic
>> header file. Is there any possible reason why it is "unsigned long" in
>> that one?
>
> It was intentional to make it unsigned long in the asm-generic
> version, based on the observation that some of the 64-bit architectures
> (alpha and parisc) were using unsigned long in their arch specific
> files. The original parisc file contained
>
> typedef struct {
>         unsigned long __softirq_pending; /* set_bit is used on this */
> } ____cacheline_aligned irq_cpustat_t;
>
> which would imply that unsigned int wouldn't work for it, and looked
> like a good idea. It turns out that the comment is outdated, set_bit
> hasn't been used on __softirq_pending on any architecture for a long
> time as far as I can tell, and 32 bits is obviously enough for it.

Yes, I just tested it on the parisc architecture.
It seems we don't even touch this variable in our code.

> The patch that Ralf just sent looks good therefore, but I'd suggest
> either reverting two of Christophs patches that changed parisc and alpha
> just to be on the safe side, or getting explicit Acks for Ralfs patch
> from the maintainers of those two architectures.

Don't revert for parisc, as either "unsigned long" or "unsigned int" is OK.
So, Ralf's patch (switching __softirq_pending back to "unsigned int") get's my Ack:
Acked-by: Helge Deller <deller@gmx.de>

Helge