[PATCH] sched/rt: hide push_irq_work_func declaration

[PATCH] sched/rt: hide push_irq_work_func declaration

[Arnd Bergmann]

The push_irq_work_func() function is conditionally defined only
when both CONFIG_SMP and HAVE_RT_PUSH_IPI are defined, but the
forward declaration remains visibile without HAVE_RT_PUSH_IPI,
causing a gcc warning in ARM64 allnoconfig:

kernel/sched/rt.c:68:13: warning: 'push_irq_work_func' declared 'static' but never defined [-Wunused-function]

This changes the code to use the same condition for both the
declaration and the function definition, which gets rid of the
warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: b6366f048e0c ("sched/rt: Use IPI to trigger RT task push migration instead of pulling")
---
Found on arm64 allnoconfig

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index e3cc16312046..ce7b36d6f477 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -64,7 +64,7 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
 	raw_spin_unlock(&rt_b->rt_runtime_lock);
 }
 
-#ifdef CONFIG_SMP
+#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
 static void push_irq_work_func(struct irq_work *work);
 #endif
 

[PATCH] sched/rt: hide push_irq_work_func declaration

[Steven Rostedt]

On Thu, 12 Nov 2015 15:22:22 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> The push_irq_work_func() function is conditionally defined only
> when both CONFIG_SMP and HAVE_RT_PUSH_IPI are defined, but the
> forward declaration remains visibile without HAVE_RT_PUSH_IPI,
> causing a gcc warning in ARM64 allnoconfig:
> 
> kernel/sched/rt.c:68:13: warning: 'push_irq_work_func' declared 'static' but never defined [-Wunused-function]
> 
> This changes the code to use the same condition for both the
> declaration and the function definition, which gets rid of the
> warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: b6366f048e0c ("sched/rt: Use IPI to trigger RT task push migration instead of pulling")
> ---
> Found on arm64 allnoconfig
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index e3cc16312046..ce7b36d6f477 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -64,7 +64,7 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
>  	raw_spin_unlock(&rt_b->rt_runtime_lock);
>  }
>  
> -#ifdef CONFIG_SMP
> +#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)

Why IS_ENABLED() and not defined()?

#if defined(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)

I thought IS_ENABLED() is used for C code, like:

	if (IS_ENABLED(CONFIG_SMP)) {
		[...]
	}

-- Steve

>  static void push_irq_work_func(struct irq_work *work);
>  #endif
>  

[PATCH] sched/rt: hide push_irq_work_func declaration

[Peter Zijlstra]

On Thu, Nov 12, 2015 at 03:22:22PM +0100, Arnd Bergmann wrote:
> -#ifdef CONFIG_SMP
> +#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
>  static void push_irq_work_func(struct irq_work *work);
>  #endif

I think we can get tid of the whole HAVE_RT_PUSH_IPI thing after:
8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")

That should allow us to use smp_calling_function_single_async() instead
of irq_work_queue_on(), removing the dependency on irq_work.

[PATCH] sched/rt: hide push_irq_work_func declaration

[Arnd Bergmann]

On Thursday 12 November 2015 09:47:39 Steven Rostedt wrote:
> > -#ifdef CONFIG_SMP
> > +#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
> 
> Why IS_ENABLED() and not defined()?
>
> #if defined(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
> 
> I thought IS_ENABLED() is used for C code, like:
> 
>         if (IS_ENABLED(CONFIG_SMP)) {
>                 [...]
>         }

"#if IS_ENABLED(CONFIG_foo)" has another property, which is
to evaluate to true when Kconfig has set the symbol to "=m".
Obviously that cannot happen for CONFIG_SMP, but some
maintainers prefer using IS_ENABLED() consistently for all
config symbols.

I don't care much either way, and it's easily changed if
we still want the patch and you prefer a plain #if defined().

	Arnd

[PATCH] sched/rt: hide push_irq_work_func declaration

[Steven Rostedt]

On Thu, 12 Nov 2015 15:53:31 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Nov 12, 2015 at 03:22:22PM +0100, Arnd Bergmann wrote:
> > -#ifdef CONFIG_SMP
> > +#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
> >  static void push_irq_work_func(struct irq_work *work);
> >  #endif  
> 
> I think we can get tid of the whole HAVE_RT_PUSH_IPI thing after:
> 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> 
> That should allow us to use smp_calling_function_single_async() instead
> of irq_work_queue_on(), removing the dependency on irq_work.

Interesting. I'll have to take a look at that.

-- Steve

[PATCH] sched/rt: hide push_irq_work_func declaration

[Steven Rostedt]

On Thu, 12 Nov 2015 16:14:06 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday 12 November 2015 09:47:39 Steven Rostedt wrote:
> > > -#ifdef CONFIG_SMP
> > > +#if IS_ENABLED(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)  
> > 
> > Why IS_ENABLED() and not defined()?
> >
> > #if defined(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
> > 
> > I thought IS_ENABLED() is used for C code, like:
> > 
> >         if (IS_ENABLED(CONFIG_SMP)) {
> >                 [...]
> >         }  
> 
> "#if IS_ENABLED(CONFIG_foo)" has another property, which is
> to evaluate to true when Kconfig has set the symbol to "=m".
> Obviously that cannot happen for CONFIG_SMP, but some
> maintainers prefer using IS_ENABLED() consistently for all
> config symbols.
> 
> I don't care much either way, and it's easily changed if
> we still want the patch and you prefer a plain #if defined().
> 

For this instance, I do prefer the "defined()" version. I can
understand the IS_ENABLED() for module configs. But I don't see the
need to make all #if's use it. Maybe because I don't deal with module
configs much.

Anyway, this patch should probably go in now as a current "fix", and
the redesign should be looked at as well, but that's unrelated to your
arch, and you shouldn't need to worry about it.

Thanks,

-- Steve

[PATCH v2] sched/rt: hide push_irq_work_func declaration

[Arnd Bergmann]

The push_irq_work_func() function is conditionally defined only
when both CONFIG_SMP and HAVE_RT_PUSH_IPI are defined, but the
forward declaration remains visibile without HAVE_RT_PUSH_IPI,
causing a gcc warning in ARM64 allnoconfig:

kernel/sched/rt.c:68:13: warning: 'push_irq_work_func' declared 'static' but never defined [-Wunused-function]

This changes the code to use the same condition for both the
declaration and the function definition, which gets rid of the
warning.

As Peter Zijlstra, we can possibly get rid of the whole HAVE_RT_PUSH_IPI
thing after:
8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")

Until that is done, this patch can be used to avoid the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: b6366f048e0c ("sched/rt: Use IPI to trigger RT task push migration instead of pulling")
---
Found on arm64 allnoconfig

v2: now using #if defined() instead IS_ENABLED()

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index e3cc16312046..ce7b36d6f477 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -64,7 +64,7 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
        raw_spin_unlock(&rt_b->rt_runtime_lock);
 }
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
 static void push_irq_work_func(struct irq_work *work);
 #endif
 

[PATCH v2] sched/rt: hide push_irq_work_func declaration

[Steven Rostedt]

On Thu, 12 Nov 2015 17:19:58 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> The push_irq_work_func() function is conditionally defined only
> when both CONFIG_SMP and HAVE_RT_PUSH_IPI are defined, but the
> forward declaration remains visibile without HAVE_RT_PUSH_IPI,
> causing a gcc warning in ARM64 allnoconfig:
> 
> kernel/sched/rt.c:68:13: warning: 'push_irq_work_func' declared 'static' but never defined [-Wunused-function]
> 
> This changes the code to use the same condition for both the
> declaration and the function definition, which gets rid of the
> warning.
> 
> As Peter Zijlstra, we can possibly get rid of the whole HAVE_RT_PUSH_IPI
> thing after:
> 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> 
> Until that is done, this patch can be used to avoid the warning.

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: b6366f048e0c ("sched/rt: Use IPI to trigger RT task push migration instead of pulling")
> ---
> Found on arm64 allnoconfig
> 
> v2: now using #if defined() instead IS_ENABLED()
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index e3cc16312046..ce7b36d6f477 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -64,7 +64,7 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
>         raw_spin_unlock(&rt_b->rt_runtime_lock);
>  }
>  
> -#ifdef CONFIG_SMP
> +#if defined(CONFIG_SMP) && defined(HAVE_RT_PUSH_IPI)
>  static void push_irq_work_func(struct irq_work *work);
>  #endif
>