Signed-off-by: Pranith Kumar <[email protected]>
---
kernel/rcu/tree.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0c47e30..67e850a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -947,12 +947,6 @@ static void print_other_cpu_stall(struct rcu_state *rsp)
force_quiescent_state(rsp); /* Kick them all. */
}
-/*
- * This function really isn't for public consumption, but RCU is special in
- * that context switches can allow the state machine to make progress.
- */
-extern void resched_cpu(int cpu);
-
static void print_cpu_stall(struct rcu_state *rsp)
{
int cpu;
--
1.7.9.5
--
Pranith
On Sun, 2014-04-13 at 21:39 -0400, Pranith Kumar wrote:
[]
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
[]
> @@ -947,12 +947,6 @@ static void print_other_cpu_stall(struct rcu_state *rsp)
> force_quiescent_state(rsp); /* Kick them all. */
> }
>
> -/*
> - * This function really isn't for public consumption, but RCU is special in
> - * that context switches can allow the state machine to make progress.
> - */
> -extern void resched_cpu(int cpu);
why not #include "sched.h"
and remove both declarations?
On Sun, Apr 13, 2014 at 10:50 PM, Joe Perches <[email protected]> wrote:
>>
>> -/*
>> - * This function really isn't for public consumption, but RCU is special in
>> - * that context switches can allow the state machine to make progress.
>> - */
>> -extern void resched_cpu(int cpu);
>
> why not #include "sched.h"
>
> and remove both declarations?
>
>
As the comment mentions, resched_cpu is internal to the scheduler and
hence is in sched/sched.h file and not in linux/sched.h.
sched/sched.h cannot be included in other subsystems directly.
Regards,
--
Pranith
On Sun, 2014-04-13 at 23:03 -0400, Pranith Kumar wrote:
> On Sun, Apr 13, 2014 at 10:50 PM, Joe Perches <[email protected]> wrote:
> >> -/*
> >> - * This function really isn't for public consumption, but RCU is special in
> >> - * that context switches can allow the state machine to make progress.
> >> - */
> >> -extern void resched_cpu(int cpu);
> >
> > why not #include "sched.h"
> > and remove both declarations?
> As the comment mentions, resched_cpu is internal to the scheduler and
> hence is in sched/sched.h file and not in linux/sched.h.
Note the use of quotes and lack of angle brackets.
> sched/sched.h cannot be included in other subsystems directly.
If functions from it can be declared extern,
then likely it could be used in an #include.
On Sun, Apr 13, 2014 at 11:18 PM, Joe Perches <[email protected]> wrote:
>> As the comment mentions, resched_cpu is internal to the scheduler and
>> hence is in sched/sched.h file and not in linux/sched.h.
>
> Note the use of quotes and lack of angle brackets.
>
>> sched/sched.h cannot be included in other subsystems directly.
>
> If functions from it can be declared extern,
> then likely it could be used in an #include.
>
>
All the users of resched_cpu are in kernel/sched/ and in
kernel/rcu/tree.c. I think the only reason this was declared extern
was for the special use in rcu/tree.c.
--
Pranith
On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
> remove duplicate definition of extern resched_cpu
>
> Signed-off-by: Pranith Kumar <[email protected]>
Hello, Pranith,
When I apply this patch I get the following:
/home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
/home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
/home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
/home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
/home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
This failed in under number of different Kconfig setups, the .config file
for one of them is attached.
So this declaration really is needed. Just out of curiosity, what led
you to believe that it could be removed?
Thanx, Paul
> ---
> kernel/rcu/tree.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 0c47e30..67e850a 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -947,12 +947,6 @@ static void print_other_cpu_stall(struct rcu_state
> *rsp)
> force_quiescent_state(rsp); /* Kick them all. */
> }
>
> -/*
> - * This function really isn't for public consumption, but RCU is special in
> - * that context switches can allow the state machine to make progress.
> - */
> -extern void resched_cpu(int cpu);
> -
> static void print_cpu_stall(struct rcu_state *rsp)
> {
> int cpu;
> --
> 1.7.9.5
>
>
> --
> Pranith
Hi Paul,
On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
<[email protected]> wrote:
> On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
>> remove duplicate definition of extern resched_cpu
>>
>> Signed-off-by: Pranith Kumar <[email protected]>
>
> Hello, Pranith,
>
> When I apply this patch I get the following:
>
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
>
> This failed in under number of different Kconfig setups, the .config file
> for one of them is attached.
>
> So this declaration really is needed. Just out of curiosity, what led
> you to believe that it could be removed?
>
That is strange. The patch removes a duplicate declaration of
resched_cpu (on lines 768, 954) of the file kernel/rcu/tree.c of the
latest git.
The patch compiles fine here with the latest tip of the git tree.
CC kernel/rcu/tree.o
Can you please check if your tree.c has two declarations for resched_cpu?
--
Pranith
On Mon, Apr 14, 2014 at 12:19 PM, Pranith Kumar <[email protected]> wrote:
> Hi Paul,
>
> On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
> <[email protected]> wrote:
>> On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
>>> remove duplicate definition of extern resched_cpu
>>>
>>> Signed-off-by: Pranith Kumar <[email protected]>
>>
>> Hello, Pranith,
>>
>> When I apply this patch I get the following:
>>
>> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
>> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
>> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
>> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
>> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
>>
>> This failed in under number of different Kconfig setups, the .config file
>> for one of them is attached.
>>
>> So this declaration really is needed. Just out of curiosity, what led
>> you to believe that it could be removed?
>>
Your attached config has the following:
Linux/x86 3.14.0-rc3 Kernel Configuration
The patch is against 3.15.0-rc1 (the latest at this time). May be that
is the reason?
--
Pranith
On Mon, Apr 14, 2014 at 12:19:31PM -0400, Pranith Kumar wrote:
> Hi Paul,
>
> On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
> <[email protected]> wrote:
> > On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
> >> remove duplicate definition of extern resched_cpu
> >>
> >> Signed-off-by: Pranith Kumar <[email protected]>
> >
> > Hello, Pranith,
> >
> > When I apply this patch I get the following:
> >
> > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
> > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
> > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
> > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
> > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
> >
> > This failed in under number of different Kconfig setups, the .config file
> > for one of them is attached.
> >
> > So this declaration really is needed. Just out of curiosity, what led
> > you to believe that it could be removed?
> >
>
> That is strange. The patch removes a duplicate declaration of
> resched_cpu (on lines 768, 954) of the file kernel/rcu/tree.c of the
> latest git.
>
> The patch compiles fine here with the latest tip of the git tree.
>
> CC kernel/rcu/tree.o
>
> Can you please check if your tree.c has two declarations for resched_cpu?
Ah, your patch didn't apply, so I hand-applied it, and removed the first
declaration rather than the second one. Trying it again.
Thanx, Paul
On Mon, 2014-04-14 at 09:32 -0700, Paul E. McKenney wrote:
> On Mon, Apr 14, 2014 at 12:19:31PM -0400, Pranith Kumar wrote:
> > Hi Paul,
> >
> > On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
> > <[email protected]> wrote:
> > > On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
> > >> remove duplicate definition of extern resched_cpu
> > >>
> > >> Signed-off-by: Pranith Kumar <[email protected]>
> > >
> > > Hello, Pranith,
> > >
> > > When I apply this patch I get the following:
> > >
> > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
> > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
> > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
> > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
> > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
> > >
> > > This failed in under number of different Kconfig setups, the .config file
> > > for one of them is attached.
> > >
> > > So this declaration really is needed. Just out of curiosity, what led
> > > you to believe that it could be removed?
> > >
> >
> > That is strange. The patch removes a duplicate declaration of
> > resched_cpu (on lines 768, 954) of the file kernel/rcu/tree.c of the
> > latest git.
> >
> > The patch compiles fine here with the latest tip of the git tree.
> >
> > CC kernel/rcu/tree.o
> >
> > Can you please check if your tree.c has two declarations for resched_cpu?
>
> Ah, your patch didn't apply, so I hand-applied it, and removed the first
> declaration rather than the second one. Trying it again.
Perhaps this might be better than using the extern.
This also would allow the resched_cpu call to become
static inline as it's very small.
---
kernel/rcu/tree.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0c47e30..7f2c8c2 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -60,6 +60,13 @@
#include "tree.h"
#include "rcu.h"
+/*
+ * This include of sched.h (for resched_cpu) really isn't for public
+ * consumption, but RCU is special in that context switches can allow
+ * the state machine to make progress.
+ */
+#include "../sched/sched.h"
+
MODULE_ALIAS("rcutree");
#ifdef MODULE_PARAM_PREFIX
#undef MODULE_PARAM_PREFIX
@@ -762,12 +769,6 @@ static int dyntick_save_progress_counter(struct rcu_data *rdp,
}
/*
- * This function really isn't for public consumption, but RCU is special in
- * that context switches can allow the state machine to make progress.
- */
-extern void resched_cpu(int cpu);
-
-/*
* Return true if the specified CPU has passed through a quiescent
* state by virtue of being in or having passed through an dynticks
* idle state since the last call to dyntick_save_progress_counter()
@@ -947,12 +948,6 @@ static void print_other_cpu_stall(struct rcu_state *rsp)
force_quiescent_state(rsp); /* Kick them all. */
}
-/*
- * This function really isn't for public consumption, but RCU is special in
- * that context switches can allow the state machine to make progress.
- */
-extern void resched_cpu(int cpu);
-
static void print_cpu_stall(struct rcu_state *rsp)
{
int cpu;
On Mon, Apr 14, 2014 at 09:53:27AM -0700, Joe Perches wrote:
> On Mon, 2014-04-14 at 09:32 -0700, Paul E. McKenney wrote:
> > On Mon, Apr 14, 2014 at 12:19:31PM -0400, Pranith Kumar wrote:
> > > Hi Paul,
> > >
> > > On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
> > > <[email protected]> wrote:
> > > > On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
> > > >> remove duplicate definition of extern resched_cpu
> > > >>
> > > >> Signed-off-by: Pranith Kumar <[email protected]>
> > > >
> > > > Hello, Pranith,
> > > >
> > > > When I apply this patch I get the following:
> > > >
> > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
> > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
> > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
> > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
> > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
> > > >
> > > > This failed in under number of different Kconfig setups, the .config file
> > > > for one of them is attached.
> > > >
> > > > So this declaration really is needed. Just out of curiosity, what led
> > > > you to believe that it could be removed?
> > > >
> > >
> > > That is strange. The patch removes a duplicate declaration of
> > > resched_cpu (on lines 768, 954) of the file kernel/rcu/tree.c of the
> > > latest git.
> > >
> > > The patch compiles fine here with the latest tip of the git tree.
> > >
> > > CC kernel/rcu/tree.o
> > >
> > > Can you please check if your tree.c has two declarations for resched_cpu?
> >
> > Ah, your patch didn't apply, so I hand-applied it, and removed the first
> > declaration rather than the second one. Trying it again.
>
> Perhaps this might be better than using the extern.
>
> This also would allow the resched_cpu call to become
> static inline as it's very small.
>
> ---
> kernel/rcu/tree.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 0c47e30..7f2c8c2 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -60,6 +60,13 @@
> #include "tree.h"
> #include "rcu.h"
>
> +/*
> + * This include of sched.h (for resched_cpu) really isn't for public
> + * consumption, but RCU is special in that context switches can allow
> + * the state machine to make progress.
> + */
> +#include "../sched/sched.h"
This sort of thing hasn't been well-received recently. Plus the call
to resched_cpu() is very infrequent, so hardly worth the optimization.
Thanx, Paul
> +
> MODULE_ALIAS("rcutree");
> #ifdef MODULE_PARAM_PREFIX
> #undef MODULE_PARAM_PREFIX
> @@ -762,12 +769,6 @@ static int dyntick_save_progress_counter(struct rcu_data *rdp,
> }
>
> /*
> - * This function really isn't for public consumption, but RCU is special in
> - * that context switches can allow the state machine to make progress.
> - */
> -extern void resched_cpu(int cpu);
> -
> -/*
> * Return true if the specified CPU has passed through a quiescent
> * state by virtue of being in or having passed through an dynticks
> * idle state since the last call to dyntick_save_progress_counter()
> @@ -947,12 +948,6 @@ static void print_other_cpu_stall(struct rcu_state *rsp)
> force_quiescent_state(rsp); /* Kick them all. */
> }
>
> -/*
> - * This function really isn't for public consumption, but RCU is special in
> - * that context switches can allow the state machine to make progress.
> - */
> -extern void resched_cpu(int cpu);
> -
> static void print_cpu_stall(struct rcu_state *rsp)
> {
> int cpu;
>
>
On Mon, Apr 14, 2014 at 12:27:47PM -0400, Pranith Kumar wrote:
> On Mon, Apr 14, 2014 at 12:19 PM, Pranith Kumar <[email protected]> wrote:
> > Hi Paul,
> >
> > On Mon, Apr 14, 2014 at 12:07 PM, Paul E. McKenney
> > <[email protected]> wrote:
> >> On Sun, Apr 13, 2014 at 05:53:53PM -0400, Pranith Kumar wrote:
> >>> remove duplicate definition of extern resched_cpu
> >>>
> >>> Signed-off-by: Pranith Kumar <[email protected]>
> >>
> >> Hello, Pranith,
> >>
> >> When I apply this patch I get the following:
> >>
> >> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_implicit_dynticks_qs’:
> >> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: error: implicit declaration of function ‘resched_cpu’ [-Werror=implicit-function-declaration]
> >> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: At top level:
> >> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:1009:13: warning: conflicting types for ‘resched_cpu’ [enabled by default]
> >> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:895:3: note: previous implicit declaration of ‘resched_cpu’ was here
> >>
> >> This failed in under number of different Kconfig setups, the .config file
> >> for one of them is attached.
> >>
> >> So this declaration really is needed. Just out of curiosity, what led
> >> you to believe that it could be removed?
> >>
>
> Your attached config has the following:
>
> Linux/x86 3.14.0-rc3 Kernel Configuration
>
> The patch is against 3.15.0-rc1 (the latest at this time). May be that
> is the reason?
Nope, it was just me messing up when hand-applying the patch. The reason
the patch wouldn't apply is that I am working against the -rcu tree at
git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git,
branch rcu/dev. Once I applied it correctly, removing the second of
the two declarations rather than the first one, it worked fine.
So please accept my apologies for my confusion. I have queued your
patch for 3.16, thank you!
Thanx, Paul