linux-next: sched tree build warning

linux-next: sched tree build warning

[Stephen Rothwell]

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

Hi all,

Today's linux-next build (powerpc ppc64_defconfig) produced this warning:

kernel/sched.c: In function 'sched_show_task':
kernel/sched.c:6677: warning: format '%08x' expects type 'unsigned int', but argument 5 has type 'long unsigned int'

Introduced by commit 12b5c43486202dd4ff3cfd59a190984a0dd7f6fd ("sched:
emit thread info flags with stack trace") from the sched tree.

thread_info::flags is "unsigned long" on all architectures except alpha
(where it is "unsigned int"), ia64 and x86 (where it is "__u32").

-- 
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: sched tree build warning

[David Rientjes]

On Thu, 7 May 2009, Stephen Rothwell wrote:

> Hi all,
> 
> Today's linux-next build (powerpc ppc64_defconfig) produced this warning:
> 
> kernel/sched.c: In function 'sched_show_task':
> kernel/sched.c:6677: warning: format '%08x' expects type 'unsigned int', but argument 5 has type 'long unsigned int'
> 
> Introduced by commit 12b5c43486202dd4ff3cfd59a190984a0dd7f6fd ("sched:
> emit thread info flags with stack trace") from the sched tree.
> 
> thread_info::flags is "unsigned long" on all architectures except alpha
> (where it is "unsigned int"), ia64 and x86 (where it is "__u32").
> 

Thanks Stephen.  There's a hacky way around this if Ingo will put up with 
it.



No architecture uses more than 32 bits for thread info flags, yet most 
declare it with type `unsigned long'.  Thus, we only show 32 bits but cast 
the variable to unsigned long for all architectures.

Signed-off-by: David Rientjes <rientjes@google.com>
---
diff --git a/kernel/sched.c b/kernel/sched.c
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -6490,9 +6490,9 @@ void sched_show_task(struct task_struct *p)
 #ifdef CONFIG_DEBUG_STACK_USAGE
 	free = stack_not_used(p);
 #endif
-	printk(KERN_CONT "%5lu %5d %6d 0x%08x\n", free,
+	printk(KERN_CONT "%5lu %5d %6d 0x%08lx\n", free,
 		task_pid_nr(p), task_pid_nr(p->real_parent),
-		task_thread_info(p)->flags);
+		(unsigned long)task_thread_info(p)->flags);
 
 	show_stack(p, NULL);
 }

Re: linux-next: sched tree build warning

[Ingo Molnar]

* David Rientjes <rientjes@google.com> wrote:

> On Thu, 7 May 2009, Stephen Rothwell wrote:
> 
> > Hi all,
> > 
> > Today's linux-next build (powerpc ppc64_defconfig) produced this warning:
> > 
> > kernel/sched.c: In function 'sched_show_task':
> > kernel/sched.c:6677: warning: format '%08x' expects type 'unsigned int', but argument 5 has type 'long unsigned int'
> > 
> > Introduced by commit 12b5c43486202dd4ff3cfd59a190984a0dd7f6fd ("sched:
> > emit thread info flags with stack trace") from the sched tree.
> > 
> > thread_info::flags is "unsigned long" on all architectures except alpha
> > (where it is "unsigned int"), ia64 and x86 (where it is "__u32").
> > 
> 
> Thanks Stephen.  There's a hacky way around this if Ingo will put 
> up with it.

> -	printk(KERN_CONT "%5lu %5d %6d 0x%08x\n", free,
> +	printk(KERN_CONT "%5lu %5d %6d 0x%08lx\n", free,
>  		task_pid_nr(p), task_pid_nr(p->real_parent),
> -		task_thread_info(p)->flags);
> +		(unsigned long)task_thread_info(p)->flags);

applied, thanks.

	Ingo

Re: linux-next: sched tree build warning

[Rusty Russell]

On Tue, 21 Apr 2009 09:57:06 am Stephen Rothwell wrote:
> Should this call to first_cpu be cpumask_first?

Always.  Old cpu ops should die.  (I didn't actually read this code,
but it's a truism).

Thanks,
Rusty.

Re: linux-next: sched tree build warning

[Ingo Molnar]

* Gautham R Shenoy <ego@in.ibm.com> wrote:

> On Tue, Apr 21, 2009 at 10:27:06AM +1000, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Today's linux-next build (powerpc ppc64_defconfig) produced this new
> > warning:
> > 
> > kernel/sched.c: In function 'find_new_ilb':
> > kernel/sched.c:4355: warning: passing argument 1 of '__first_cpu' from incompatible pointer type
> > 
> > Possibly caused by commit f711f6090a81cbd396b63de90f415d33f563af9b
> > ("sched: Nominate idle load balancer from a semi-idle package") from the
> > sched tree.  Should this call to first_cpu be cpumask_first?
> 
> Yes, it should be cpumask_first. Patch appended.
> 
> -->
> sched: Replace first_cpu() with cpumask_first() in ILB nomination code.
> 
> From: Gautham R Shenoy <ego@in.ibm.com>
> 
> For !(CONFIG_SCHED_MC || CONFIG_SCHED_SMT), find_new_ilb() nominates the
> Idle load balancer as the first cpu from the nohz.cpu_mask.
> 
> This code uses the older API first_cpu(). Replace it with cpumask_first(),
> which is the correct API here.
> 
> Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> ---
> 
>  kernel/sched.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index bbf367d..6df8dd5 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -4344,7 +4344,7 @@ out_done:
>  #else /*  (CONFIG_SCHED_MC || CONFIG_SCHED_SMT) */
>  static inline int find_new_ilb(int call_cpu)
>  {
> -	return first_cpu(nohz.cpu_mask);
> +	return cpumask_first(nohz.cpu_mask);

Given that this is not an onstack cpumask the type mismatch is 
harmless in effect but obviously needs fixed.

I've applied your fix to tip:sched/core - thanks guys.

	Ingo

Re: linux-next: sched tree build warning

[Stephen Rothwell]

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

Hi Gautham,

On Tue, 21 Apr 2009 08:40:49 +0530 Gautham R Shenoy <ego@in.ibm.com> wrote:
>
> Yes, it should be cpumask_first. Patch appended.

Thanks.

-- 
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: sched tree build warning

[Gautham R Shenoy]

On Tue, Apr 21, 2009 at 10:27:06AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc64_defconfig) produced this new
> warning:
> 
> kernel/sched.c: In function 'find_new_ilb':
> kernel/sched.c:4355: warning: passing argument 1 of '__first_cpu' from incompatible pointer type
> 
> Possibly caused by commit f711f6090a81cbd396b63de90f415d33f563af9b
> ("sched: Nominate idle load balancer from a semi-idle package") from the
> sched tree.  Should this call to first_cpu be cpumask_first?

Yes, it should be cpumask_first. Patch appended.

-->
sched: Replace first_cpu() with cpumask_first() in ILB nomination code.

From: Gautham R Shenoy <ego@in.ibm.com>

For !(CONFIG_SCHED_MC || CONFIG_SCHED_SMT), find_new_ilb() nominates the
Idle load balancer as the first cpu from the nohz.cpu_mask.

This code uses the older API first_cpu(). Replace it with cpumask_first(),
which is the correct API here.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---

 kernel/sched.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/kernel/sched.c b/kernel/sched.c
index bbf367d..6df8dd5 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4344,7 +4344,7 @@ out_done:
 #else /*  (CONFIG_SCHED_MC || CONFIG_SCHED_SMT) */
 static inline int find_new_ilb(int call_cpu)
 {
-	return first_cpu(nohz.cpu_mask);
+	return cpumask_first(nohz.cpu_mask);
 }
 #endif
 

> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/



-- 
Thanks and Regards
gautham

linux-next: sched tree build warning

[Stephen Rothwell]

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

Hi all,

Today's linux-next build (powerpc ppc64_defconfig) produced this new
warning:

kernel/sched.c: In function 'find_new_ilb':
kernel/sched.c:4355: warning: passing argument 1 of '__first_cpu' from incompatible pointer type

Possibly caused by commit f711f6090a81cbd396b63de90f415d33f563af9b
("sched: Nominate idle load balancer from a semi-idle package") from the
sched tree.  Should this call to first_cpu be cpumask_first?

-- 
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: sched tree build warning

[Paul Mackerras]

Ingo Molnar writes:

> okay, so what's the timeline to double check those details and to get this 
> fix upstream?

Dunno - I have sent a query to our glibc guys, but it is vacation
season, which may delay things...

Paul.

Re: linux-next: sched tree build warning

[Ingo Molnar]

* Paul Mackerras <paulus@samba.org> wrote:

> > I cannot see how the binary representation could ever change from 
> > this. (and that is all that an ABI is about - it is an application 
> > Binary interface. I.e. there's no ABI breakage.)
> 
> Yes, the bits are the same, but that doesn't mean the types are the 
> same.  And we do export type definitions.
> 
> I once wanted to change the ppc32 size_t definition from unsigned int to 
> unsigned long to match up with ppc64.  That caused more pain than it was 
> worth because of exactly this issue (and also because gcc has fixed 
> ideas about size_t) so I abandoned it.  That's why I'm cautious about 
> changing user-visible types.  I'm not saying we can't do it, I'm saying 
> we shouldn't do it unilaterally.

okay, so what's the timeline to double check those details and to get this 
fix upstream?

	Ingo

Re: linux-next: sched tree build warning

[Paul Mackerras]

Ingo Molnar writes:

> which APIs do you mean exactly, could you give me an example please and 
> the type of breakage you suspect?

Any struct with a __u64 or __s64 in it that gets exported to userland
will have a different type signature with the change, hence has the
potential to cause compile warnings or errors on correct code
(e.g. warnings on printf's that use %ld to print an __s64 field on
ppc64).  It's possible that C++ stuff will fail to link because of
mangled names coming out differently.  And so on.

> I cannot see how the binary representation could ever change from this. 
> (and that is all that an ABI is about - it is an application Binary 
> interface. I.e. there's no ABI breakage.)

Yes, the bits are the same, but that doesn't mean the types are the
same.  And we do export type definitions.

I once wanted to change the ppc32 size_t definition from unsigned int
to unsigned long to match up with ppc64.  That caused more pain than
it was worth because of exactly this issue (and also because gcc has
fixed ideas about size_t) so I abandoned it.  That's why I'm cautious
about changing user-visible types.  I'm not saying we can't do it, I'm
saying we shouldn't do it unilaterally.

Paul.

Re: linux-next: sched tree build warning

[Ingo Molnar]

* Paul Mackerras <paulus@samba.org> wrote:

> Ingo Molnar writes:
> 
> > the real solution is something like the patch below. That generates new 
> > (but harmless) warnings within the powerpc code but those are a one-off 
> > effort to fix and are not reoccuring.
> > 
> > Cc:-ed Paul Mackerras - Paul, am i missing anything?
> 
> That does change the formal types of things exported to userland, and 
> hence technically breaks the ABI, which is why I am cautious about this 
> idea.  Whether or not that causes real problems in practice I'm not 
> sure, but I would want to at least check with the glibc developers 
> first.
> 
> One solution to might be to use an #ifdef __KERNEL__ so that userland 
> still sees the long types but kernel code sees long longs.

which APIs do you mean exactly, could you give me an example please and 
the type of breakage you suspect?

I cannot see how the binary representation could ever change from this. 
(and that is all that an ABI is about - it is an application Binary 
interface. I.e. there's no ABI breakage.)

	Ingo

Re: linux-next: sched tree build warning

[Paul Mackerras]

Ingo Molnar writes:

> the real solution is something like the patch below. That generates new 
> (but harmless) warnings within the powerpc code but those are a one-off 
> effort to fix and are not reoccuring.
> 
> Cc:-ed Paul Mackerras - Paul, am i missing anything?

That does change the formal types of things exported to userland, and
hence technically breaks the ABI, which is why I am cautious about
this idea.  Whether or not that causes real problems in practice I'm
not sure, but I would want to at least check with the glibc developers
first.

One solution to might be to use an #ifdef __KERNEL__ so that userland
still sees the long types but kernel code sees long longs.

Paul.

Re: linux-next: sched tree build warning

[Stephen Rothwell]

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

Hi Ingo,

On Mon, 22 Dec 2008 08:04:26 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> the real solution is something like the patch below. That generates new 
> (but harmless) warnings within the powerpc code but those are a one-off 
> effort to fix and are not reoccuring.
> 
> Cc:-ed Paul Mackerras - Paul, am i missing anything?

You mean besides the patches that fix all those warnings? :-)

-- 
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: sched tree build warning

[Ingo Molnar]

* Ken Chen <kenchen@google.com> wrote:

> On Sun, Dec 21, 2008 at 8:22 PM, Stephen Rothwell wrote:
> > Hi all,
> >
> > Today's linux-next build (powerpc ppc64_defconfig) produced this new
> > warning:
> >
> > fs/proc/base.c: In function 'proc_pid_schedstat':
> > fs/proc/base.c:352: warning: format '%llu' expects type 'long long unsigned int',
> > but argument 3 has type 'u64'
> >
> > Introduced by commit 9c2c48020ec0dd6ecd27e5a1298f73b40d85a595
> > ("schedstat: consolidate per-task cpu runtime stats").
> 
> oh boy, this is the 2nd time that printing u64 bite me, I really need to 
> get hold of a machine that is non-x86_64.
> 
> Just saw Ingo's patch came in while I'm typing this email.  Thank you 
> Ingo for fixing this.

the real solution is something like the patch below. That generates new 
(but harmless) warnings within the powerpc code but those are a one-off 
effort to fix and are not reoccuring.

Cc:-ed Paul Mackerras - Paul, am i missing anything?

	Ingo

--------------->
>From 3d835af97da5b6cb755ca4f8464f8e636cc321da Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Mon, 22 Dec 2008 08:02:06 +0100
Subject: [PATCH] powerpc: fix u64

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/powerpc/include/asm/types.h |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
index c004c13..1cbfd7f 100644
--- a/arch/powerpc/include/asm/types.h
+++ b/arch/powerpc/include/asm/types.h
@@ -1,11 +1,7 @@
 #ifndef _ASM_POWERPC_TYPES_H
 #define _ASM_POWERPC_TYPES_H
 
-#ifdef __powerpc64__
-# include <asm-generic/int-l64.h>
-#else
-# include <asm-generic/int-ll64.h>
-#endif
+#include <asm-generic/int-ll64.h>
 
 #ifndef __ASSEMBLY__
 

Re: linux-next: sched tree build warning

[Ken Chen]

On Sun, Dec 21, 2008 at 8:22 PM, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next build (powerpc ppc64_defconfig) produced this new
> warning:
>
> fs/proc/base.c: In function 'proc_pid_schedstat':
> fs/proc/base.c:352: warning: format '%llu' expects type 'long long unsigned int',
> but argument 3 has type 'u64'
>
> Introduced by commit 9c2c48020ec0dd6ecd27e5a1298f73b40d85a595
> ("schedstat: consolidate per-task cpu runtime stats").

oh boy, this is the 2nd time that printing u64 bite me, I really need
to get hold of a machine that is non-x86_64.

Just saw Ingo's patch came in while I'm typing this email.  Thank you
Ingo for fixing this.

- Ken

Re: linux-next: sched tree build warning

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (powerpc ppc64_defconfig) produced this new
> warning:
> 
> fs/proc/base.c: In function 'proc_pid_schedstat':
> fs/proc/base.c:352: warning: format '%llu' expects type 'long long unsigned int', but argument 3 has type 'u64'
> 
> Introduced by commit 9c2c48020ec0dd6ecd27e5a1298f73b40d85a595
> ("schedstat: consolidate per-task cpu runtime stats").

that should be harmless - fix below.

Given that all these warnings generated by asm-generic/int-l64.h are 
bogus, all the time - the right and clean fix is to change the 
architectures that use it (s390, mips, ia64, sparc, powerpc, alpha) to 
asm-generic/int-ll64.h. It will break nothing - there will only be a 
one-time conversion to avoid similar build warnings.

	Ingo

----------------->
>From 826e08b0157c0ce8a80dfe3c0a6c5a1540dd0b1d Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Mon, 22 Dec 2008 07:37:41 +0100
Subject: [PATCH] sched: fix warning in fs/proc/base.c

Stephen Rothwell reported this new (harmless) build warning on platforms that
define u64 to long:

 fs/proc/base.c: In function 'proc_pid_schedstat':
 fs/proc/base.c:352: warning: format '%llu' expects type 'long long unsigned int', but argument 3 has type 'u64'

asm-generic/int-l64.h platforms strike again: that file should be eliminated.

Fix it by casting the parameters to long long.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 fs/proc/base.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 4d745ba..a48200c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -347,8 +347,8 @@ static int proc_pid_wchan(struct task_struct *task, char *buffer)
 static int proc_pid_schedstat(struct task_struct *task, char *buffer)
 {
 	return sprintf(buffer, "%llu %llu %lu\n",
-			task->se.sum_exec_runtime,
-			task->sched_info.run_delay,
+			(unsigned long long)task->se.sum_exec_runtime,
+			(unsigned long long)task->sched_info.run_delay,
 			task->sched_info.pcount);
 }
 #endif

linux-next: sched tree build warning

[Stephen Rothwell]

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

Hi all,

Today's linux-next build (powerpc ppc64_defconfig) produced this new
warning:

fs/proc/base.c: In function 'proc_pid_schedstat':
fs/proc/base.c:352: warning: format '%llu' expects type 'long long unsigned int', but argument 3 has type 'u64'

Introduced by commit 9c2c48020ec0dd6ecd27e5a1298f73b40d85a595
("schedstat: consolidate per-task cpu runtime stats").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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