2016-11-21 04:57:27

by yjin

[permalink] [raw]
Subject: [PATCH] powerpc: cputime: fix a compile warning

From: Yanjiang Jin <[email protected]>

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 <[email protected]>
---
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


2016-11-30 14:41:08

by Crystal Wood

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning

On Mon, 2016-11-21 at 12:56 +0800, [email protected] wrote:
> From: Yanjiang Jin <[email protected]>
>
> 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 <[email protected]>
> ---
>  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

2016-12-01 05:05:28

by yjin

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning

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, [email protected] wrote:
>> From: Yanjiang Jin <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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
>


Attachments:
0001-powerpc-cputime-fix-a-compile-warning.patch (1.11 kB)

2016-12-02 04:15:04

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning

[email protected] 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

2016-12-02 04:23:01

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning

On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman <[email protected]> wrote:
> [email protected] 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

2016-12-02 04:36:24

by yjin

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning


On 2016年12月02日 12:22, Balbir Singh wrote:
> On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman <[email protected]> wrote:
>> [email protected] 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

2016-12-02 05:16:03

by Pan Xinhui

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning



在 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 <[email protected]> wrote:
>>> [email protected] 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>

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
>

2016-12-02 05:18:27

by yjin

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning


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
>>> <[email protected]> wrote:
>>>> [email protected] 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Link:
> http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
>
> 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
>>
>

2016-12-02 19:55:07

by Crystal Wood

[permalink] [raw]
Subject: Re: [PATCH] powerpc: cputime: fix a compile warning

On Fri, 2016-12-02 at 15:15 +1100, Michael Ellerman wrote:
> [email protected] 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