Hello,
I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
Thanks,
Byungchul
Changes from v1
-. WARN_ON_ONCE() on failing to create rcu_boost_kthread.
-. Changed title and commit message a bit.
---8<---
From 7100fcf82202f063f70f45def208ea5198412f5a Mon Sep 17 00:00:00 2001
From: Byungchul Park <[email protected]>
Date: Thu, 27 Jun 2019 15:58:10 +0900
Subject: [PATCH v2] rcu: Change return type of rcu_spawn_one_boost_kthread()
The return value of rcu_spawn_one_boost_kthread() is not used any
longer. Change return type of that function from int to void.
Signed-off-by: Byungchul Park <[email protected]>
---
kernel/rcu/tree_plugin.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 1102765..3c8444e 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1131,7 +1131,7 @@ static void rcu_preempt_boost_start_gp(struct rcu_node *rnp)
* already exist. We only create this kthread for preemptible RCU.
* Returns zero if all is well, a negated errno otherwise.
*/
-static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
+static void rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
{
int rnp_index = rnp - rcu_get_root();
unsigned long flags;
@@ -1139,25 +1139,24 @@ static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
struct task_struct *t;
if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
- return 0;
+ return;
if (!rcu_scheduler_fully_active || rcu_rnp_online_cpus(rnp) == 0)
- return 0;
+ return;
rcu_state.boost = 1;
if (rnp->boost_kthread_task != NULL)
- return 0;
+ return;
t = kthread_create(rcu_boost_kthread, (void *)rnp,
"rcub/%d", rnp_index);
- if (IS_ERR(t))
- return PTR_ERR(t);
+ if (WARN_ON_ONCE(IS_ERR(t)))
+ return;
raw_spin_lock_irqsave_rcu_node(rnp, flags);
rnp->boost_kthread_task = t;
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
sp.sched_priority = kthread_prio;
sched_setscheduler_nocheck(t, SCHED_FIFO, &sp);
wake_up_process(t); /* get to TASK_INTERRUPTIBLE quickly. */
- return 0;
}
static void rcu_cpu_kthread_setup(unsigned int cpu)
@@ -1265,7 +1264,7 @@ static void __init rcu_spawn_boost_kthreads(void)
if (WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcub kthread, OOM is now expected behavior\n", __func__))
return;
rcu_for_each_leaf_node(rnp)
- (void)rcu_spawn_one_boost_kthread(rnp);
+ rcu_spawn_one_boost_kthread(rnp);
}
static void rcu_prepare_kthreads(int cpu)
@@ -1275,7 +1274,7 @@ static void rcu_prepare_kthreads(int cpu)
/* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
if (rcu_scheduler_fully_active)
- (void)rcu_spawn_one_boost_kthread(rnp);
+ rcu_spawn_one_boost_kthread(rnp);
}
#else /* #ifdef CONFIG_RCU_BOOST */
--
1.9.1
On Thu, Jun 27, 2019 at 04:07:46PM +0900, Byungchul Park wrote:
> Hello,
>
> I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
Looks pretty safe to me and nice clean up!
Acked-by: Joel Fernandes (Google) <[email protected]>
- Joel
>
> Thanks,
> Byungchul
>
> Changes from v1
> -. WARN_ON_ONCE() on failing to create rcu_boost_kthread.
> -. Changed title and commit message a bit.
>
> ---8<---
> From 7100fcf82202f063f70f45def208ea5198412f5a Mon Sep 17 00:00:00 2001
> From: Byungchul Park <[email protected]>
> Date: Thu, 27 Jun 2019 15:58:10 +0900
> Subject: [PATCH v2] rcu: Change return type of rcu_spawn_one_boost_kthread()
>
> The return value of rcu_spawn_one_boost_kthread() is not used any
> longer. Change return type of that function from int to void.
>
> Signed-off-by: Byungchul Park <[email protected]>
> ---
> kernel/rcu/tree_plugin.h | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 1102765..3c8444e 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1131,7 +1131,7 @@ static void rcu_preempt_boost_start_gp(struct rcu_node *rnp)
> * already exist. We only create this kthread for preemptible RCU.
> * Returns zero if all is well, a negated errno otherwise.
> */
> -static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> +static void rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> {
> int rnp_index = rnp - rcu_get_root();
> unsigned long flags;
> @@ -1139,25 +1139,24 @@ static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> struct task_struct *t;
>
> if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
> - return 0;
> + return;
>
> if (!rcu_scheduler_fully_active || rcu_rnp_online_cpus(rnp) == 0)
> - return 0;
> + return;
>
> rcu_state.boost = 1;
> if (rnp->boost_kthread_task != NULL)
> - return 0;
> + return;
> t = kthread_create(rcu_boost_kthread, (void *)rnp,
> "rcub/%d", rnp_index);
> - if (IS_ERR(t))
> - return PTR_ERR(t);
> + if (WARN_ON_ONCE(IS_ERR(t)))
> + return;
> raw_spin_lock_irqsave_rcu_node(rnp, flags);
> rnp->boost_kthread_task = t;
> raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> sp.sched_priority = kthread_prio;
> sched_setscheduler_nocheck(t, SCHED_FIFO, &sp);
> wake_up_process(t); /* get to TASK_INTERRUPTIBLE quickly. */
> - return 0;
> }
>
> static void rcu_cpu_kthread_setup(unsigned int cpu)
> @@ -1265,7 +1264,7 @@ static void __init rcu_spawn_boost_kthreads(void)
> if (WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcub kthread, OOM is now expected behavior\n", __func__))
> return;
> rcu_for_each_leaf_node(rnp)
> - (void)rcu_spawn_one_boost_kthread(rnp);
> + rcu_spawn_one_boost_kthread(rnp);
> }
>
> static void rcu_prepare_kthreads(int cpu)
> @@ -1275,7 +1274,7 @@ static void rcu_prepare_kthreads(int cpu)
>
> /* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
> if (rcu_scheduler_fully_active)
> - (void)rcu_spawn_one_boost_kthread(rnp);
> + rcu_spawn_one_boost_kthread(rnp);
> }
>
> #else /* #ifdef CONFIG_RCU_BOOST */
> --
> 1.9.1
>
On Thu, Jun 27, 2019 at 09:42:40AM -0400, Joel Fernandes wrote:
> On Thu, Jun 27, 2019 at 04:07:46PM +0900, Byungchul Park wrote:
> > Hello,
> >
> > I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
>
> Looks pretty safe to me and nice clean up!
>
> Acked-by: Joel Fernandes (Google) <[email protected]>
Agreed, but I still cannot find where this applies. I did try rcu/next,
which is currently here:
commit b989ff070574ad8b8621d866de0a8e9a65d42c80 (rcu/rcu/next, rcu/next)
Merge: 4289ee7d5a83 11ca7a9d541d
Author: Paul E. McKenney <[email protected]>
Date: Mon Jun 24 09:12:39 2019 -0700
Merge LKMM and RCU commits
Help?
Thanx, Paul
> - Joel
>
> >
> > Thanks,
> > Byungchul
> >
> > Changes from v1
> > -. WARN_ON_ONCE() on failing to create rcu_boost_kthread.
> > -. Changed title and commit message a bit.
> >
> > ---8<---
> > From 7100fcf82202f063f70f45def208ea5198412f5a Mon Sep 17 00:00:00 2001
> > From: Byungchul Park <[email protected]>
> > Date: Thu, 27 Jun 2019 15:58:10 +0900
> > Subject: [PATCH v2] rcu: Change return type of rcu_spawn_one_boost_kthread()
> >
> > The return value of rcu_spawn_one_boost_kthread() is not used any
> > longer. Change return type of that function from int to void.
> >
> > Signed-off-by: Byungchul Park <[email protected]>
> > ---
> > kernel/rcu/tree_plugin.h | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 1102765..3c8444e 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -1131,7 +1131,7 @@ static void rcu_preempt_boost_start_gp(struct rcu_node *rnp)
> > * already exist. We only create this kthread for preemptible RCU.
> > * Returns zero if all is well, a negated errno otherwise.
> > */
> > -static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > +static void rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > {
> > int rnp_index = rnp - rcu_get_root();
> > unsigned long flags;
> > @@ -1139,25 +1139,24 @@ static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > struct task_struct *t;
> >
> > if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
> > - return 0;
> > + return;
> >
> > if (!rcu_scheduler_fully_active || rcu_rnp_online_cpus(rnp) == 0)
> > - return 0;
> > + return;
> >
> > rcu_state.boost = 1;
> > if (rnp->boost_kthread_task != NULL)
> > - return 0;
> > + return;
> > t = kthread_create(rcu_boost_kthread, (void *)rnp,
> > "rcub/%d", rnp_index);
> > - if (IS_ERR(t))
> > - return PTR_ERR(t);
> > + if (WARN_ON_ONCE(IS_ERR(t)))
> > + return;
> > raw_spin_lock_irqsave_rcu_node(rnp, flags);
> > rnp->boost_kthread_task = t;
> > raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> > sp.sched_priority = kthread_prio;
> > sched_setscheduler_nocheck(t, SCHED_FIFO, &sp);
> > wake_up_process(t); /* get to TASK_INTERRUPTIBLE quickly. */
> > - return 0;
> > }
> >
> > static void rcu_cpu_kthread_setup(unsigned int cpu)
> > @@ -1265,7 +1264,7 @@ static void __init rcu_spawn_boost_kthreads(void)
> > if (WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcub kthread, OOM is now expected behavior\n", __func__))
> > return;
> > rcu_for_each_leaf_node(rnp)
> > - (void)rcu_spawn_one_boost_kthread(rnp);
> > + rcu_spawn_one_boost_kthread(rnp);
> > }
> >
> > static void rcu_prepare_kthreads(int cpu)
> > @@ -1275,7 +1274,7 @@ static void rcu_prepare_kthreads(int cpu)
> >
> > /* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
> > if (rcu_scheduler_fully_active)
> > - (void)rcu_spawn_one_boost_kthread(rnp);
> > + rcu_spawn_one_boost_kthread(rnp);
> > }
> >
> > #else /* #ifdef CONFIG_RCU_BOOST */
> > --
> > 1.9.1
> >
On Thu, Jun 27, 2019 at 01:57:03PM -0700, Paul E. McKenney wrote:
> On Thu, Jun 27, 2019 at 09:42:40AM -0400, Joel Fernandes wrote:
> > On Thu, Jun 27, 2019 at 04:07:46PM +0900, Byungchul Park wrote:
> > > Hello,
> > >
> > > I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
> >
> > Looks pretty safe to me and nice clean up!
> >
> > Acked-by: Joel Fernandes (Google) <[email protected]>
>
> Agreed, but I still cannot find where this applies. I did try rcu/next,
> which is currently here:
>
> commit b989ff070574ad8b8621d866de0a8e9a65d42c80 (rcu/rcu/next, rcu/next)
> Merge: 4289ee7d5a83 11ca7a9d541d
> Author: Paul E. McKenney <[email protected]>
> Date: Mon Jun 24 09:12:39 2019 -0700
>
> Merge LKMM and RCU commits
>
> Help?
commit 204d7a60670f3f6399a4d0826667ab7863b3e429
Merge LKMM and RCU commits
I made it on top of the above. And could you tell me which branch I'd
better use when developing. I think it's been changing sometimes.
Thank you for the answer in advance!
Thanks,
Byungchul
> Thanx, Paul
>
> > - Joel
> >
> > >
> > > Thanks,
> > > Byungchul
> > >
> > > Changes from v1
> > > -. WARN_ON_ONCE() on failing to create rcu_boost_kthread.
> > > -. Changed title and commit message a bit.
> > >
> > > ---8<---
> > > From 7100fcf82202f063f70f45def208ea5198412f5a Mon Sep 17 00:00:00 2001
> > > From: Byungchul Park <[email protected]>
> > > Date: Thu, 27 Jun 2019 15:58:10 +0900
> > > Subject: [PATCH v2] rcu: Change return type of rcu_spawn_one_boost_kthread()
> > >
> > > The return value of rcu_spawn_one_boost_kthread() is not used any
> > > longer. Change return type of that function from int to void.
> > >
> > > Signed-off-by: Byungchul Park <[email protected]>
> > > ---
> > > kernel/rcu/tree_plugin.h | 17 ++++++++---------
> > > 1 file changed, 8 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > > index 1102765..3c8444e 100644
> > > --- a/kernel/rcu/tree_plugin.h
> > > +++ b/kernel/rcu/tree_plugin.h
> > > @@ -1131,7 +1131,7 @@ static void rcu_preempt_boost_start_gp(struct rcu_node *rnp)
> > > * already exist. We only create this kthread for preemptible RCU.
> > > * Returns zero if all is well, a negated errno otherwise.
> > > */
> > > -static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > > +static void rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > > {
> > > int rnp_index = rnp - rcu_get_root();
> > > unsigned long flags;
> > > @@ -1139,25 +1139,24 @@ static int rcu_spawn_one_boost_kthread(struct rcu_node *rnp)
> > > struct task_struct *t;
> > >
> > > if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
> > > - return 0;
> > > + return;
> > >
> > > if (!rcu_scheduler_fully_active || rcu_rnp_online_cpus(rnp) == 0)
> > > - return 0;
> > > + return;
> > >
> > > rcu_state.boost = 1;
> > > if (rnp->boost_kthread_task != NULL)
> > > - return 0;
> > > + return;
> > > t = kthread_create(rcu_boost_kthread, (void *)rnp,
> > > "rcub/%d", rnp_index);
> > > - if (IS_ERR(t))
> > > - return PTR_ERR(t);
> > > + if (WARN_ON_ONCE(IS_ERR(t)))
> > > + return;
> > > raw_spin_lock_irqsave_rcu_node(rnp, flags);
> > > rnp->boost_kthread_task = t;
> > > raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> > > sp.sched_priority = kthread_prio;
> > > sched_setscheduler_nocheck(t, SCHED_FIFO, &sp);
> > > wake_up_process(t); /* get to TASK_INTERRUPTIBLE quickly. */
> > > - return 0;
> > > }
> > >
> > > static void rcu_cpu_kthread_setup(unsigned int cpu)
> > > @@ -1265,7 +1264,7 @@ static void __init rcu_spawn_boost_kthreads(void)
> > > if (WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcub kthread, OOM is now expected behavior\n", __func__))
> > > return;
> > > rcu_for_each_leaf_node(rnp)
> > > - (void)rcu_spawn_one_boost_kthread(rnp);
> > > + rcu_spawn_one_boost_kthread(rnp);
> > > }
> > >
> > > static void rcu_prepare_kthreads(int cpu)
> > > @@ -1275,7 +1274,7 @@ static void rcu_prepare_kthreads(int cpu)
> > >
> > > /* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
> > > if (rcu_scheduler_fully_active)
> > > - (void)rcu_spawn_one_boost_kthread(rnp);
> > > + rcu_spawn_one_boost_kthread(rnp);
> > > }
> > >
> > > #else /* #ifdef CONFIG_RCU_BOOST */
> > > --
> > > 1.9.1
> > >
On Fri, Jun 28, 2019 at 11:43:39AM +0900, Byungchul Park wrote:
> On Thu, Jun 27, 2019 at 01:57:03PM -0700, Paul E. McKenney wrote:
> > On Thu, Jun 27, 2019 at 09:42:40AM -0400, Joel Fernandes wrote:
> > > On Thu, Jun 27, 2019 at 04:07:46PM +0900, Byungchul Park wrote:
> > > > Hello,
> > > >
> > > > I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
> > >
> > > Looks pretty safe to me and nice clean up!
> > >
> > > Acked-by: Joel Fernandes (Google) <[email protected]>
> >
> > Agreed, but I still cannot find where this applies. I did try rcu/next,
> > which is currently here:
> >
> > commit b989ff070574ad8b8621d866de0a8e9a65d42c80 (rcu/rcu/next, rcu/next)
> > Merge: 4289ee7d5a83 11ca7a9d541d
> > Author: Paul E. McKenney <[email protected]>
> > Date: Mon Jun 24 09:12:39 2019 -0700
> >
> > Merge LKMM and RCU commits
> >
> > Help?
>
> commit 204d7a60670f3f6399a4d0826667ab7863b3e429
>
> Merge LKMM and RCU commits
>
> I made it on top of the above. And could you tell me which branch I'd
> better use when developing. I think it's been changing sometimes.
That would be because idiot here took so much care to avoid risking
pushing some early development commits into the upcoming merge window
that he managed to misplace them entirely. The -rcu tree's "dev" branch
now includes them. Could you please port to it?
a1af11a24cb0 ("rcu/nocb: Make __call_rcu_nocb_wake() safe for many callbacks")
> Thank you for the answer in advance!
And please accept my apologies for the very confusing tree layout this
time around!
Thanx, Paul
On Sun, Jun 30, 2019 at 12:38:34PM -0700, Paul E. McKenney wrote:
> On Fri, Jun 28, 2019 at 11:43:39AM +0900, Byungchul Park wrote:
> > On Thu, Jun 27, 2019 at 01:57:03PM -0700, Paul E. McKenney wrote:
> > > On Thu, Jun 27, 2019 at 09:42:40AM -0400, Joel Fernandes wrote:
> > > > On Thu, Jun 27, 2019 at 04:07:46PM +0900, Byungchul Park wrote:
> > > > > Hello,
> > > > >
> > > > > I tested if the WARN_ON_ONCE() is fired with my box and it was ok.
> > > >
> > > > Looks pretty safe to me and nice clean up!
> > > >
> > > > Acked-by: Joel Fernandes (Google) <[email protected]>
> > >
> > > Agreed, but I still cannot find where this applies. I did try rcu/next,
> > > which is currently here:
> > >
> > > commit b989ff070574ad8b8621d866de0a8e9a65d42c80 (rcu/rcu/next, rcu/next)
> > > Merge: 4289ee7d5a83 11ca7a9d541d
> > > Author: Paul E. McKenney <[email protected]>
> > > Date: Mon Jun 24 09:12:39 2019 -0700
> > >
> > > Merge LKMM and RCU commits
> > >
> > > Help?
> >
> > commit 204d7a60670f3f6399a4d0826667ab7863b3e429
> >
> > Merge LKMM and RCU commits
> >
> > I made it on top of the above. And could you tell me which branch I'd
> > better use when developing. I think it's been changing sometimes.
>
> That would be because idiot here took so much care to avoid risking
> pushing some early development commits into the upcoming merge window
> that he managed to misplace them entirely. The -rcu tree's "dev" branch
> now includes them. Could you please port to it?
Of course, I can.
> a1af11a24cb0 ("rcu/nocb: Make __call_rcu_nocb_wake() safe for many callbacks")
>
> > Thank you for the answer in advance!
>
> And please accept my apologies for the very confusing tree layout this
> time around!
It would be totally OK if you give me the answer when I feel confused
with branch for development. :)
>
> Thanx, Paul