[PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[Geert Uytterhoeven]

On Mon, Feb 1, 2010 at 08:12, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Today's linux-next build (powerpc allnoconfig) produced this warning:
>
> kernel/sched.c: In function 'wake_up_new_task':
> kernel/sched.c:2631: warning: unused variable 'cpu'
>
> Introduced by commit fabf318e5e4bda0aca2b0d617b191884fda62703 ("sched:
> Fix fork vs hotplug vs cpuset namespaces").

And now we have it in 2.6.33-rc7, too...
Patch below (FWIW, compile-tested on m68k with CONFIG_SMP=n only).

---
>From cbf4f334632ade9c5ed9b88728ec82af074e4ace Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Sun, 7 Feb 2010 20:47:30 +0100
Subject: [PATCH] sched: Fix unused variable warning on UP

Fix warning

| kernel/sched.c:2650: warning: unused variable 'cpu'

if CONFIG_SMP is not set.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 kernel/sched.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3a8fb30..c47561e 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2647,9 +2647,10 @@ void wake_up_new_task(struct task_struct *p,
unsigned long clone_flags)
 {
 	unsigned long flags;
 	struct rq *rq;
-	int cpu = get_cpu();

 #ifdef CONFIG_SMP
+	int cpu = get_cpu();
+
 	/*
 	 * Fork balancing, do it here and not earlier because:
 	 *  - cpus_allowed can change in the fork path
-- 
1.6.0.4

-- 
Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[Peter Zijlstra]

On Sun, 2010-02-07 at 20:57 +0100, Geert Uytterhoeven wrote:
> On Mon, Feb 1, 2010 at 08:12, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Today's linux-next build (powerpc allnoconfig) produced this warning:
> >
> > kernel/sched.c: In function 'wake_up_new_task':
> > kernel/sched.c:2631: warning: unused variable 'cpu'
> >
> > Introduced by commit fabf318e5e4bda0aca2b0d617b191884fda62703 ("sched:
> > Fix fork vs hotplug vs cpuset namespaces").
> 
> And now we have it in 2.6.33-rc7, too...
> Patch below (FWIW, compile-tested on m68k with CONFIG_SMP=n only).
> 
> ---
> From cbf4f334632ade9c5ed9b88728ec82af074e4ace Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Date: Sun, 7 Feb 2010 20:47:30 +0100
> Subject: [PATCH] sched: Fix unused variable warning on UP
> 
> Fix warning
> 
> | kernel/sched.c:2650: warning: unused variable 'cpu'
> 
> if CONFIG_SMP is not set.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  kernel/sched.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3a8fb30..c47561e 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -2647,9 +2647,10 @@ void wake_up_new_task(struct task_struct *p,
> unsigned long clone_flags)
>  {
>  	unsigned long flags;
>  	struct rq *rq;
> -	int cpu = get_cpu();
> 
>  #ifdef CONFIG_SMP
> +	int cpu = get_cpu();
> +
>  	/*
>  	 * Fork balancing, do it here and not earlier because:
>  	 *  - cpus_allowed can change in the fork path

Which introduces a preempt imbalance... I like akpm's fix much better.

Re: [PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[Geert Uytterhoeven]

On Sun, Feb 7, 2010 at 20:58, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, 2010-02-07 at 20:57 +0100, Geert Uytterhoeven wrote:
>> On Mon, Feb 1, 2010 at 08:12, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> > Today's linux-next build (powerpc allnoconfig) produced this warning:
>> >
>> > kernel/sched.c: In function 'wake_up_new_task':
>> > kernel/sched.c:2631: warning: unused variable 'cpu'
>> >
>> > Introduced by commit fabf318e5e4bda0aca2b0d617b191884fda62703 ("sched:
>> > Fix fork vs hotplug vs cpuset namespaces").
>>
>> And now we have it in 2.6.33-rc7, too...
>> Patch below (FWIW, compile-tested on m68k with CONFIG_SMP=n only).
>>
>> ---
>> From cbf4f334632ade9c5ed9b88728ec82af074e4ace Mon Sep 17 00:00:00 2001
>> From: Geert Uytterhoeven <geert@linux-m68k.org>
>> Date: Sun, 7 Feb 2010 20:47:30 +0100
>> Subject: [PATCH] sched: Fix unused variable warning on UP
>>
>> Fix warning
>>
>> | kernel/sched.c:2650: warning: unused variable 'cpu'
>>
>> if CONFIG_SMP is not set.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>  kernel/sched.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/sched.c b/kernel/sched.c
>> index 3a8fb30..c47561e 100644
>> --- a/kernel/sched.c
>> +++ b/kernel/sched.c
>> @@ -2647,9 +2647,10 @@ void wake_up_new_task(struct task_struct *p,
>> unsigned long clone_flags)
>>  {
>>       unsigned long flags;
>>       struct rq *rq;
>> -     int cpu = get_cpu();
>>
>>  #ifdef CONFIG_SMP
>> +     int cpu = get_cpu();
>> +
>>       /*
>>        * Fork balancing, do it here and not earlier because:
>>        *  - cpus_allowed can change in the fork path
>
> Which introduces a preempt imbalance... I like akpm's fix much better.

Bummer, you're right. Sorry, that teaches me not to write patches
after 2 days of FOSDEM ;-)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[Geert Uytterhoeven]

On Sun, Feb 7, 2010 at 21:26, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Feb 7, 2010 at 20:58, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Sun, 2010-02-07 at 20:57 +0100, Geert Uytterhoeven wrote:
>>> On Mon, Feb 1, 2010 at 08:12, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>> > Today's linux-next build (powerpc allnoconfig) produced this warning:
>>> >
>>> > kernel/sched.c: In function 'wake_up_new_task':
>>> > kernel/sched.c:2631: warning: unused variable 'cpu'
>>> >
>>> > Introduced by commit fabf318e5e4bda0aca2b0d617b191884fda62703 ("sched:
>>> > Fix fork vs hotplug vs cpuset namespaces").
>>>
>>> And now we have it in 2.6.33-rc7, too...
>>> Patch below (FWIW, compile-tested on m68k with CONFIG_SMP=n only).
>>>
>>> ---
>>> From cbf4f334632ade9c5ed9b88728ec82af074e4ace Mon Sep 17 00:00:00 2001
>>> From: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Date: Sun, 7 Feb 2010 20:47:30 +0100
>>> Subject: [PATCH] sched: Fix unused variable warning on UP
>>>
>>> Fix warning
>>>
>>> | kernel/sched.c:2650: warning: unused variable 'cpu'
>>>
>>> if CONFIG_SMP is not set.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>>  kernel/sched.c |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kernel/sched.c b/kernel/sched.c
>>> index 3a8fb30..c47561e 100644
>>> --- a/kernel/sched.c
>>> +++ b/kernel/sched.c
>>> @@ -2647,9 +2647,10 @@ void wake_up_new_task(struct task_struct *p,
>>> unsigned long clone_flags)
>>>  {
>>>       unsigned long flags;
>>>       struct rq *rq;
>>> -     int cpu = get_cpu();
>>>
>>>  #ifdef CONFIG_SMP
>>> +     int cpu = get_cpu();
>>> +
>>>       /*
>>>        * Fork balancing, do it here and not earlier because:
>>>        *  - cpus_allowed can change in the fork path
>>
>> Which introduces a preempt imbalance... I like akpm's fix much better.
>
> Bummer, you're right. Sorry, that teaches me not to write patches
> after 2 days of FOSDEM ;-)

<trying to completely destroy my credibility>
Shouldn't put_cpu() take a (possibly dummy) `cpu' parameter, as
returned by get_cpu()?
</trying>

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[Andrew Morton]

On Sun, 7 Feb 2010 21:31:56 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Shouldn't put_cpu() take a (possibly dummy) `cpu' parameter, as
> returned by get_cpu()?

Yeah, that always seemed a bit screwy.  Something like this...

--- a/include/linux/smp.h~a
+++ a/include/linux/smp.h
@@ -177,6 +177,15 @@ smp_call_function_any(const struct cpuma
 #define put_cpu()		preempt_enable()
 
 /*
+ * This just exists to touch the `cpu' arg, to suppress unused var
+ * warnings
+ */
+static inline void put_cpu_nr(unsigned cpu)
+{
+	put_cpu();
+}
+
+/*
  * Callback to arch code if there's nosmp or maxcpus=0 on the
  * boot command line:
  */
_

Re: [PATCH] sched: Fix unused variable warning on UP (was: Re: linux-next: tip tree build warning)

[H. Peter Anvin]

On 02/07/2010 12:31 PM, Geert Uytterhoeven wrote:
>
> <trying to completely destroy my credibility>
> Shouldn't put_cpu() take a (possibly dummy) `cpu' parameter, as
> returned by get_cpu()?
> </trying>
>

Please tell me that you're joking.

There is absolutely no reason for it, since there is only one CPU that 
you can put (the one you're already on.)  The only thing that will 
happen if you insist on carrying the CPU number forward (unless it is 
used anyway) is that the compiler will generate worse code.

get_cpu() returning the CPU you're on is a convenience; it's so you 
don't need to do a third operation just to get your current CPU number, 
but it could just as easily be done that way.

	-hpa