[PATCH] powerpc: cputime: fix a compile warning

[PATCH] powerpc: cputime: fix a compile warning

[yanjiang.jin]

From: Yanjiang Jin <yanjiang.jin@windriver.com>

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin <yanjiang.jin@windriver.com>
---
 arch/powerpc/include/asm/cputime.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
 	return (__force cputime_t) ct;
 }
 
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)	\
+	(__force u64)(cputime_to_clock_t((cputime_t)(ct)))
 
 /*
  * PPC64 uses PACA which is task independent for storing accounting data while
-- 
1.9.1

Re: [PATCH] powerpc: cputime: fix a compile warning

[Scott Wood]

On Mon, 2016-11-21 at 12:56 +0800, yanjiang.jin@windriver.com wrote:
> From: Yanjiang Jin <yanjiang.jin@windriver.com>
> 
> This patch is to avoid the below warning:
> 
> kernel/sched/cpuacct.c:298:25: warning:
> format '%lld' expects argument of type 'long long int',
> but argument 4 has type 'long unsigned int' [-Wformat=]
> 
> Signed-off-by: Yanjiang Jin <yanjiang.jin@windriver.com>
> ---
>  arch/powerpc/include/asm/cputime.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/cputime.h
> b/arch/powerpc/include/asm/cputime.h
> index 4f60db0..4423e97 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const
> unsigned long clk)
>  	return (__force cputime_t) ct;
>  }
>  
> -#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
> +#define cputime64_to_clock_t(ct)	\
> +	(__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Why is __force needed?

-Scott

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

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

Hi Scott,

Thanks for your reminder!
I rephrased it as below:

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)       \
+       (u64)(cputime_to_clock_t((cputime_t)(ct)))


and rebuilt the kernel, no warnings found.
New patch is attached, could you also help me to review it?

Regards!
Yanjiang

On 2016年11月30日 22:40, Scott Wood wrote:
> On Mon, 2016-11-21 at 12:56 +0800, yanjiang.jin@windriver.com wrote:
>> From: Yanjiang Jin <yanjiang.jin@windriver.com>
>>
>> This patch is to avoid the below warning:
>>
>> kernel/sched/cpuacct.c:298:25: warning:
>> format '%lld' expects argument of type 'long long int',
>> but argument 4 has type 'long unsigned int' [-Wformat=]
>>
>> Signed-off-by: Yanjiang Jin <yanjiang.jin@windriver.com>
>> ---
>>   arch/powerpc/include/asm/cputime.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/cputime.h
>> b/arch/powerpc/include/asm/cputime.h
>> index 4f60db0..4423e97 100644
>> --- a/arch/powerpc/include/asm/cputime.h
>> +++ b/arch/powerpc/include/asm/cputime.h
>> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const
>> unsigned long clk)
>>   	return (__force cputime_t) ct;
>>   }
>>   
>> -#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
>> +#define cputime64_to_clock_t(ct)	\
>> +	(__force u64)(cputime_to_clock_t((cputime_t)(ct)))
> Why is __force needed?
>
> -Scott
>


[-- Attachment #2: 0001-powerpc-cputime-fix-a-compile-warning.patch --]
[-- Type: text/x-patch, Size: 1137 bytes --]

>From c32bc6ffe070e3d79a420e6adcbed5709079dc23 Mon Sep 17 00:00:00 2001
From: Yanjiang Jin <yanjiang.jin@windriver.com>
Date: Sat, 19 Nov 2016 05:04:53 +0000
Subject: [PATCH] powerpc: cputime: fix a compile warning

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin <yanjiang.jin@windriver.com>
---
 arch/powerpc/include/asm/cputime.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 4f60db0..a25082b 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
 	return (__force cputime_t) ct;
 }
 
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)	\
+	(u64)(cputime_to_clock_t((cputime_t)(ct)))
 
 /*
  * PPC64 uses PACA which is task independent for storing accounting data while
-- 
1.9.1

Re: [PATCH] powerpc: cputime: fix a compile warning

[Michael Ellerman]

yanjiang.jin@windriver.com writes:

> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
> index 4f60db0..4423e97 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
>  	return (__force cputime_t) ct;
>  }
>  
> -#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
> +#define cputime64_to_clock_t(ct)	\
> +	(__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Given the name of the function is "cputime64 to clock_t", surely we
should be returning a clock_t ?

cheers

Re: [PATCH] powerpc: cputime: fix a compile warning

[Balbir Singh]

On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> yanjiang.jin@windriver.com writes:
>
>> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
>> index 4f60db0..4423e97 100644
>> --- a/arch/powerpc/include/asm/cputime.h
>> +++ b/arch/powerpc/include/asm/cputime.h
>> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
>>       return (__force cputime_t) ct;
>>  }
>>
>> -#define cputime64_to_clock_t(ct)     cputime_to_clock_t((cputime_t)(ct))
>> +#define cputime64_to_clock_t(ct)     \
>> +     (__force u64)(cputime_to_clock_t((cputime_t)(ct)))
>
> Given the name of the function is "cputime64 to clock_t", surely we
> should be returning a clock_t ?

Please fix it in cpuacct.c  Also check out git commit
527b0a76f41d062381adbb55c8eb61e32cb0bfc9
sched/cpuacct: Avoid %lld seq_printf warning

Balbir

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 12:22, Balbir Singh wrote:
> On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> yanjiang.jin@windriver.com writes:
>>
>>> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
>>> index 4f60db0..4423e97 100644
>>> --- a/arch/powerpc/include/asm/cputime.h
>>> +++ b/arch/powerpc/include/asm/cputime.h
>>> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
>>>        return (__force cputime_t) ct;
>>>   }
>>>
>>> -#define cputime64_to_clock_t(ct)     cputime_to_clock_t((cputime_t)(ct))
>>> +#define cputime64_to_clock_t(ct)     \
>>> +     (__force u64)(cputime_to_clock_t((cputime_t)(ct)))
>> Given the name of the function is "cputime64 to clock_t", surely we
>> should be returning a clock_t ?
> Please fix it in cpuacct.c  Also check out git commit
> 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
> sched/cpuacct: Avoid %lld seq_printf warning

Hi Balbir,

Where can I find this commit?

Thanks!
Yanjiang
>
> Balbir

Re: [PATCH] powerpc: cputime: fix a compile warning

[Pan Xinhui]

在 2016/12/2 12:35, yjin 写道:
>
> On 2016年12月02日 12:22, Balbir Singh wrote:
>> On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> yanjiang.jin@windriver.com writes:
>>>
>>>> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
>>>> index 4f60db0..4423e97 100644
>>>> --- a/arch/powerpc/include/asm/cputime.h
>>>> +++ b/arch/powerpc/include/asm/cputime.h
>>>> @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
>>>>        return (__force cputime_t) ct;
>>>>   }
>>>>
>>>> -#define cputime64_to_clock_t(ct)     cputime_to_clock_t((cputime_t)(ct))
>>>> +#define cputime64_to_clock_t(ct)     \
>>>> +     (__force u64)(cputime_to_clock_t((cputime_t)(ct)))
>>> Given the name of the function is "cputime64 to clock_t", surely we
>>> should be returning a clock_t ?
>> Please fix it in cpuacct.c  Also check out git commit
>> 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
>> sched/cpuacct: Avoid %lld seq_printf warning
>
> Hi Balbir,
>
> Where can I find this commit?
>
hello,
	it is in next tree. :)

commit 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date:   Fri Nov 11 15:27:49 2016 +0100

     sched/cpuacct: Avoid %lld seq_printf warning
     
     For s390 kernel builds I keep getting this warning:
     
      kernel/sched/cpuacct.c: In function 'cpuacct_stats_show':
      kernel/sched/cpuacct.c:298:25: warning: format '%lld' expects argument of type 'long long int', but argument 4 has type 'clock_t {aka long int}' [-Wformat=]
        seq_printf(sf, "%s %lld\n",
     
     Silence the warning by adding an explicit cast.
     
     Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
     Cc: Linus Torvalds <torvalds@linux-foundation.org>
     Cc: Peter Zijlstra <peterz@infradead.org>
     Cc: Thomas Gleixner <tglx@linutronix.de>
     Link: http://lkml.kernel.org/r/20161111142749.6545-1-schwidefsky@de.ibm.com
     Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index bc0b309c..9add206 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -297,7 +297,7 @@ static int cpuacct_stats_show(struct seq_file *sf, void *v)
         for (stat = 0; stat < CPUACCT_STAT_NSTATS; stat++) {
                 seq_printf(sf, "%s %lld\n",
                            cpuacct_stat_desc[stat],
-                          cputime64_to_clock_t(val[stat]));
+                          (long long)cputime64_to_clock_t(val[stat]));
         }
  
         return 0;

> Thanks!
> Yanjiang
>>
>> Balbir
>

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 13:15, Pan Xinhui wrote:
>
>
> 在 2016/12/2 12:35, yjin 写道:
>>
>> On 2016年12月02日 12:22, Balbir Singh wrote:
>>> On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman 
>>> <mpe@ellerman.id.au> wrote:
>>>> yanjiang.jin@windriver.com writes:
>>>>
>>>>> diff --git a/arch/powerpc/include/asm/cputime.h 
>>>>> b/arch/powerpc/include/asm/cputime.h
>>>>> index 4f60db0..4423e97 100644
>>>>> --- a/arch/powerpc/include/asm/cputime.h
>>>>> +++ b/arch/powerpc/include/asm/cputime.h
>>>>> @@ -228,7 +228,8 @@ static inline cputime_t 
>>>>> clock_t_to_cputime(const unsigned long clk)
>>>>>        return (__force cputime_t) ct;
>>>>>   }
>>>>>
>>>>> -#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
>>>>> +#define cputime64_to_clock_t(ct)     \
>>>>> +     (__force u64)(cputime_to_clock_t((cputime_t)(ct)))
>>>> Given the name of the function is "cputime64 to clock_t", surely we
>>>> should be returning a clock_t ?
>>> Please fix it in cpuacct.c  Also check out git commit
>>> 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
>>> sched/cpuacct: Avoid %lld seq_printf warning
>>
>> Hi Balbir,
>>
>> Where can I find this commit?
>>
> hello,
>     it is in next tree. :)

Got it. Thanks!

Regards!
Yanjiang

>
> commit 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
> Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date:   Fri Nov 11 15:27:49 2016 +0100
>
>     sched/cpuacct: Avoid %lld seq_printf warning
>         For s390 kernel builds I keep getting this warning:
>          kernel/sched/cpuacct.c: In function 'cpuacct_stats_show':
>      kernel/sched/cpuacct.c:298:25: warning: format '%lld' expects 
> argument of type 'long long int', but argument 4 has type 'clock_t 
> {aka long int}' [-Wformat=]
>        seq_printf(sf, "%s %lld\n",
>         Silence the warning by adding an explicit cast.
>         Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
>     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>     Cc: Linus Torvalds <torvalds@linux-foundation.org>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Thomas Gleixner <tglx@linutronix.de>
>     Link: 
> http://lkml.kernel.org/r/20161111142749.6545-1-schwidefsky@de.ibm.com
>     Signed-off-by: Ingo Molnar <mingo@kernel.org>
>
> diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
> index bc0b309c..9add206 100644
> --- a/kernel/sched/cpuacct.c
> +++ b/kernel/sched/cpuacct.c
> @@ -297,7 +297,7 @@ static int cpuacct_stats_show(struct seq_file *sf, 
> void *v)
>         for (stat = 0; stat < CPUACCT_STAT_NSTATS; stat++) {
>                 seq_printf(sf, "%s %lld\n",
>                            cpuacct_stat_desc[stat],
> -                          cputime64_to_clock_t(val[stat]));
> +                          (long long)cputime64_to_clock_t(val[stat]));
>         }
>
>         return 0;
>
>> Thanks!
>> Yanjiang
>>>
>>> Balbir
>>
>

Re: [PATCH] powerpc: cputime: fix a compile warning

[Scott Wood]

On Fri, 2016-12-02 at 15:15 +1100, Michael Ellerman wrote:
> yanjiang.jin@windriver.com writes:
> 
> > 
> > diff --git a/arch/powerpc/include/asm/cputime.h
> > b/arch/powerpc/include/asm/cputime.h
> > index 4f60db0..4423e97 100644
> > --- a/arch/powerpc/include/asm/cputime.h
> > +++ b/arch/powerpc/include/asm/cputime.h
> > @@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const
> > unsigned long clk)
> >  	return (__force cputime_t) ct;
> >  }
> >  
> > -#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct
> > ))
> > +#define cputime64_to_clock_t(ct)	\
> > +	(__force u64)(cputime_to_clock_t((cputime_t)(ct)))
> Given the name of the function is "cputime64 to clock_t", surely we
> should be returning a clock_t ?

That was my initial reaction but it seems that this function has meant "return
a u64 that is otherwise like clock_t" since before the beginning of git
history.  Both generic implementations return u64, including
jiffies_64_to_clock_t which does so explicitly.

-Scott