[PATCH v2] sched: Fix compiler warnings

Commit 143e1e28cb (sched: Rework sched_domain topology definition)
introduced a number of functions with a return value of 'const int'.
gcc doesn't know what to do with that and, if the kernel is compiled
with W=1, complains with the following warnings whenever sched.h
is included.

include/linux/sched.h:875:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:882:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:889:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:1002:21: warning:
	type qualifiers ignored on function return type

Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table)
and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce
the same warning in the arm and powerpc code.

Drop 'const' from the function declarations to fix the problem.

The fix for all three patches has to be applied together to avoid
compilation failures for the affected architectures.

Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Fix problem in all affected architectures with a single patch
    to avoid compilation errors.

 arch/arm/kernel/topology.c | 2 +-
 arch/powerpc/kernel/smp.c  | 2 +-
 include/linux/sched.h      | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 9d85318..e35d880 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c

@@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid)
 		cpu_topology[cpuid].socket_id, mpidr);
 }
 
-static inline const int cpu_corepower_flags(void)
+static inline int cpu_corepower_flags(void)
 {
 	return SD_SHARE_PKG_RESOURCES  | SD_SHARE_POWERDOMAIN;
 }

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 51a3ff7..1007fb8 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c

@@ -747,7 +747,7 @@ int setup_profiling_timer(unsigned int multiplier)
 
 #ifdef CONFIG_SCHED_SMT
 /* cpumask of CPUs with asymetric SMT dependancy */
-static const int powerpc_smt_flags(void)
+static int powerpc_smt_flags(void)
 {
 	int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
 

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 306f4f0..0376b05 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h

@@ -872,21 +872,21 @@ enum cpu_idle_type {
 #define SD_NUMA			0x4000	/* cross-node balancing */
 
 #ifdef CONFIG_SCHED_SMT
-static inline const int cpu_smt_flags(void)
+static inline int cpu_smt_flags(void)
 {
 	return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
 }
 #endif
 
 #ifdef CONFIG_SCHED_MC
-static inline const int cpu_core_flags(void)
+static inline int cpu_core_flags(void)
 {
 	return SD_SHARE_PKG_RESOURCES;
 }
 #endif
 
 #ifdef CONFIG_NUMA
-static inline const int cpu_numa_flags(void)
+static inline int cpu_numa_flags(void)
 {
 	return SD_NUMA;
 }

@@ -999,7 +999,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
 bool cpus_share_cache(int this_cpu, int that_cpu);
 
 typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
-typedef const int (*sched_domain_flags_f)(void);
+typedef int (*sched_domain_flags_f)(void);
 
 #define SDTL_OVERLAP	0x01
 

-- 
1.9.1

On Tue, 2014-06-24 at 18:05 -0700, Guenter Roeck wrote:

 .../...

Drop 'const' from the function declarations to fix the problem.

The fix for all three patches has to be applied together to avoid
compilation failures for the affected architectures.

Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Cc: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Fix problem in all affected architectures with a single patch
    to avoid compilation errors.

On 25 June 2014 03:05, Guenter Roeck [off-list ref] wrote:

Commit 143e1e28cb (sched: Rework sched_domain topology definition)
introduced a number of functions with a return value of 'const int'.
gcc doesn't know what to do with that and, if the kernel is compiled
with W=1, complains with the following warnings whenever sched.h
is included.

include/linux/sched.h:875:25: warning:
        type qualifiers ignored on function return type
include/linux/sched.h:882:25: warning:
        type qualifiers ignored on function return type
include/linux/sched.h:889:25: warning:
        type qualifiers ignored on function return type
include/linux/sched.h:1002:21: warning:
        type qualifiers ignored on function return type

Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table)
and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce
the same warning in the arm and powerpc code.

Drop 'const' from the function declarations to fix the problem.

The fix for all three patches has to be applied together to avoid
compilation failures for the affected architectures.

Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>

Acked-by Vincent Guittot [off-list ref]

On Tue, Jun 24, 2014 at 06:05:29PM -0700, Guenter Roeck wrote:

Maybe the author's intention was:

	static inline int cpu_corepower_flags(void) __attribute__((const));

?
This specifies that the function has no side effects and the return value
only depends on the (here non-existing) function arguments.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

On 06/25/2014 12:14 AM, Uwe Kleine-König wrote:

On Tue, Jun 24, 2014 at 06:05:29PM -0700, Guenter Roeck wrote:

quoted

Commit 143e1e28cb (sched: Rework sched_domain topology definition)
introduced a number of functions with a return value of 'const int'.
gcc doesn't know what to do with that and, if the kernel is compiled
with W=1, complains with the following warnings whenever sched.h
is included.

include/linux/sched.h:875:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:882:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:889:25: warning:
	type qualifiers ignored on function return type
include/linux/sched.h:1002:21: warning:
	type qualifiers ignored on function return type

Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table)
and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce
the same warning in the arm and powerpc code.

Drop 'const' from the function declarations to fix the problem.

The fix for all three patches has to be applied together to avoid
compilation failures for the affected architectures.

Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Fix problem in all affected architectures with a single patch
     to avoid compilation errors.

  arch/arm/kernel/topology.c | 2 +-
  arch/powerpc/kernel/smp.c  | 2 +-
  include/linux/sched.h      | 8 ++++----
  3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 9d85318..e35d880 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c

@@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid)
  		cpu_topology[cpuid].socket_id, mpidr);
  }

-static inline const int cpu_corepower_flags(void)
+static inline int cpu_corepower_flags(void)
  {
  	return SD_SHARE_PKG_RESOURCES  | SD_SHARE_POWERDOMAIN;
  }

Maybe the author's intention was:

	static inline int cpu_corepower_flags(void) __attribute__((const));

?
This specifies that the function has no side effects and the return value
only depends on the (here non-existing) function arguments.

Possibly, but either I am missing something or this doesn't compile.

Guenter

Hello Guenter,

On Wed, Jun 25, 2014 at 07:27:47AM -0700, Guenter Roeck wrote:

quoted

Maybe the author's intention was:

static inline int cpu_corepower_flags(void) __attribute__((const));

?
This specifies that the function has no side effects and the return value
only depends on the (here non-existing) function arguments.

Possibly, but either I am missing something or this doesn't compile.

You need to do a separate declaration:

	static inline int cpu_corepower_flags(void) __attribute__((const));
	static inline int cpu_corepower_flags(void)
	{
		...

Does this help?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

On 06/25/2014 07:49 AM, Uwe Kleine-König wrote:

Hello Guenter,

On Wed, Jun 25, 2014 at 07:27:47AM -0700, Guenter Roeck wrote:

quoted

quoted

Maybe the author's intention was:

	static inline int cpu_corepower_flags(void) __attribute__((const));

?
This specifies that the function has no side effects and the return value
only depends on the (here non-existing) function arguments.

Possibly, but either I am missing something or this doesn't compile.

You need to do a separate declaration:

	static inline int cpu_corepower_flags(void) __attribute__((const));
	static inline int cpu_corepower_flags(void)
	{
		...

Actually turns out one can use __attribute_const__, and it is

	static inline int __attribute_const__ cpu_corepower_flags(void)

which turns out to be widely used.

I'll change that and resubmit after testing.

Guenter

RnJvbTogR3VlbnRlciBSb2Vjaw0KPiBPbiAwNi8yNS8yMDE0IDA3OjQ5IEFNLCBVd2UgS2xlaW5l
LUtuaWcgd3JvdGU6DQo+ID4gSGVsbG8gR3VlbnRlciwNCj4gPg0KPiA+IE9uIFdlZCwgSnVuIDI1
LCAyMDE0IGF0IDA3OjI3OjQ3QU0gLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3JvdGU6DQo+ID4+PiBN
YXliZSB0aGUgYXV0aG9yJ3MgaW50ZW50aW9uIHdhczoNCj4gPj4+DQo+ID4+PiAJc3RhdGljIGlu
bGluZSBpbnQgY3B1X2NvcmVwb3dlcl9mbGFncyh2b2lkKSBfX2F0dHJpYnV0ZV9fKChjb25zdCkp
Ow0KPiA+Pj4NCj4gPj4+ID8NCj4gPj4+IFRoaXMgc3BlY2lmaWVzIHRoYXQgdGhlIGZ1bmN0aW9u
IGhhcyBubyBzaWRlIGVmZmVjdHMgYW5kIHRoZSByZXR1cm4gdmFsdWUNCj4gPj4+IG9ubHkgZGVw
ZW5kcyBvbiB0aGUgKGhlcmUgbm9uLWV4aXN0aW5nKSBmdW5jdGlvbiBhcmd1bWVudHMuDQo+ID4+
Pg0KPiA+Pg0KPiA+PiBQb3NzaWJseSwgYnV0IGVpdGhlciBJIGFtIG1pc3Npbmcgc29tZXRoaW5n
IG9yIHRoaXMgZG9lc24ndCBjb21waWxlLg0KPiA+IFlvdSBuZWVkIHRvIGRvIGEgc2VwYXJhdGUg
ZGVjbGFyYXRpb246DQo+ID4NCj4gPiAJc3RhdGljIGlubGluZSBpbnQgY3B1X2NvcmVwb3dlcl9m
bGFncyh2b2lkKSBfX2F0dHJpYnV0ZV9fKChjb25zdCkpOw0KPiA+IAlzdGF0aWMgaW5saW5lIGlu
dCBjcHVfY29yZXBvd2VyX2ZsYWdzKHZvaWQpDQo+ID4gCXsNCj4gPiAJCS4uLg0KPiANCj4gQWN0
dWFsbHkgdHVybnMgb3V0IG9uZSBjYW4gdXNlIF9fYXR0cmlidXRlX2NvbnN0X18sIGFuZCBpdCBp
cw0KPiANCj4gCXN0YXRpYyBpbmxpbmUgaW50IF9fYXR0cmlidXRlX2NvbnN0X18gY3B1X2NvcmVw
b3dlcl9mbGFncyh2b2lkKQ0KPiANCj4gd2hpY2ggdHVybnMgb3V0IHRvIGJlIHdpZGVseSB1c2Vk
Lg0KPiANCj4gSSdsbCBjaGFuZ2UgdGhhdCBhbmQgcmVzdWJtaXQgYWZ0ZXIgdGVzdGluZy4NCg0K
WW91IGRvbid0IG5lZWQgdG8gdGVsbCB0aGUgY29tcGlsZXIgdGhhdCBmb3IgYW4gaW5saW5lIGZ1
bmN0aW9uLg0KDQoJRGF2aWQNCg0K

Hello,

On Wed, Jun 25, 2014 at 03:40:28PM +0000, David Laight wrote:

From: Guenter Roeck

quoted

Actually turns out one can use __attribute_const__, and it is

	static inline int __attribute_const__ cpu_corepower_flags(void)

which turns out to be widely used.

I'll change that and resubmit after testing.

You don't need to tell the compiler that for an inline function.

I didn't check for the functions in question here, but in general your
statement is wrong.

For example:

static inline unsigned int __attribute_const__ read_cpuid_id(void)
{
	return readl(BASEADDR_V7M_SCB + V7M_SCB_CPUID);
}

from arch/arm/include/asm/cputype.h. The V7M_SCB_CPUID register never
changes, but there is no way gcc can deduce that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

From: Uwe Kleine-K=F6nig=20

Hello,
=20
On Wed, Jun 25, 2014 at 03:40:28PM +0000, David Laight wrote:

quoted

From: Guenter Roeck

quoted

Actually turns out one can use __attribute_const__, and it is

	static inline int __attribute_const__ cpu_corepower_flags(void)

which turns out to be widely used.

I'll change that and resubmit after testing.

You don't need to tell the compiler that for an inline function.

I didn't check for the functions in question here, but in general your
statement is wrong.
=20
For example:
=20
static inline unsigned int __attribute_const__ read_cpuid_id(void)
{
	return readl(BASEADDR_V7M_SCB + V7M_SCB_CPUID);
}
=20
from arch/arm/include/asm/cputype.h. The V7M_SCB_CPUID register never
changes, but there is no way gcc can deduce that.

Hmm... it all rather depends on the order of the optimisations and=20
inlining.

I've tried to use 'restrict' on the parameters to an inline function
in an attempt to get 'noalias' - but the reverse inference never
seems to be applied.

	David

On 06/25/2014 08:52 AM, Uwe Kleine-König wrote:

Hello,

On Wed, Jun 25, 2014 at 03:40:28PM +0000, David Laight wrote:

quoted

From: Guenter Roeck

quoted

Actually turns out one can use __attribute_const__, and it is

	static inline int __attribute_const__ cpu_corepower_flags(void)

which turns out to be widely used.

I'll change that and resubmit after testing.

You don't need to tell the compiler that for an inline function.

I didn't check for the functions in question here, but in general your
statement is wrong.

For example:

static inline unsigned int __attribute_const__ read_cpuid_id(void)
{
	return readl(BASEADDR_V7M_SCB + V7M_SCB_CPUID);
}

from arch/arm/include/asm/cputype.h. The V7M_SCB_CPUID register never
changes, but there is no way gcc can deduce that.

Sigh. As I mentioned earlier, it is much easier to introduce a problem
than to fix it.

Ok, I'll leave this alone. I already spent much more time on this than
I should or have, so it is really time to move on.

Guenter

Hi Guenter,

[I know I'm a bit late to this, but ...]

On Tue, 24 Jun 2014 18:05:29 -0700 Guenter Roeck [off-list ref] wrote:

The only reference to this function is to take its address, so "inline"
is useless, right?

The same is true of those three, but then they would have to be moved
into a .c file and replaced with prototypes ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

On 06/25/2014 05:59 PM, Stephen Rothwell wrote:

Hi Guenter,

[I know I'm a bit late to this, but ...]

On Tue, 24 Jun 2014 18:05:29 -0700 Guenter Roeck [off-list ref] wrote:

quoted

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 9d85318..e35d880 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c

@@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid)
  		cpu_topology[cpuid].socket_id, mpidr);
  }

-static inline const int cpu_corepower_flags(void)
+static inline int cpu_corepower_flags(void)
  {
  	return SD_SHARE_PKG_RESOURCES  | SD_SHARE_POWERDOMAIN;
  }

The only reference to this function is to take its address, so "inline"
is useless, right?

quoted

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 306f4f0..0376b05 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h

@@ -872,21 +872,21 @@ enum cpu_idle_type {
  #define SD_NUMA			0x4000	/* cross-node balancing */

  #ifdef CONFIG_SCHED_SMT
-static inline const int cpu_smt_flags(void)
+static inline int cpu_smt_flags(void)
  {
  	return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
  }
  #endif

  #ifdef CONFIG_SCHED_MC
-static inline const int cpu_core_flags(void)
+static inline int cpu_core_flags(void)
  {
  	return SD_SHARE_PKG_RESOURCES;
  }
  #endif

  #ifdef CONFIG_NUMA
-static inline const int cpu_numa_flags(void)
+static inline int cpu_numa_flags(void)
  {
  	return SD_NUMA;
  }

The same is true of those three, but then they would have to be moved
into a .c file and replaced with prototypes ...

Personally I wasn't sure why it had to be functions instead of defines,
but who knows. Anyway, seems others are not happy with my proposed fix
either, and everyone seems to suggest a different solution, so I guess
it won't go anywhere.

I "solved" my immediate problem of getting a polluted build log by
filtering the warnings out, so I don't really care too much anymore ;-).

Guenter