[8/8] sched: Relax the set_cpus_allowed_ptr() semantics
diff mbox series

Message ID 20210116113920.187137770@infradead.org
State New, archived
Headers show
Series
  • sched: Fix hot-unplug regressions
Related show

Commit Message

Peter Zijlstra Jan. 16, 2021, 11:30 a.m. UTC
Now that we have KTHREAD_IS_PER_CPU to denote the critical per-cpu
tasks to retain during CPU offline, we can relax the warning in
set_cpus_allowed_ptr(). Any spurious kthread that wants to get on at
the last minute will get pushed off before it can run.

While during CPU online there is no harm, and actual benefit, to
allowing kthreads back on early, it simplifies hotplug code.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

Comments

Lai Jiangshan Jan. 16, 2021, 2:39 p.m. UTC | #1
On Sat, Jan 16, 2021 at 7:43 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Now that we have KTHREAD_IS_PER_CPU to denote the critical per-cpu
> tasks to retain during CPU offline, we can relax the warning in
> set_cpus_allowed_ptr(). Any spurious kthread that wants to get on at
> the last minute will get pushed off before it can run.
>
> While during CPU online there is no harm, and actual benefit, to
> allowing kthreads back on early, it simplifies hotplug code.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Thanks!

Relaxing set_cpus_allowed_ptr() was also one of the choices I listed,
which can really simplify hotplug code in the workqueue and may be
other hotplug code.

Reviewed-by: Lai jiangshan <jiangshanlai@gmail.com>

> ---
>  kernel/sched/core.c |   20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2342,7 +2342,9 @@ static int __set_cpus_allowed_ptr(struct
>
>         if (p->flags & PF_KTHREAD || is_migration_disabled(p)) {
>                 /*
> -                * Kernel threads are allowed on online && !active CPUs.
> +                * Kernel threads are allowed on online && !active CPUs,
> +                * however, during cpu-hot-unplug, even these might get pushed
> +                * away if not KTHREAD_IS_PER_CPU.
>                  *
>                  * Specifically, migration_disabled() tasks must not fail the
>                  * cpumask_any_and_distribute() pick below, esp. so on
> @@ -2386,16 +2388,6 @@ static int __set_cpus_allowed_ptr(struct
>
>         __do_set_cpus_allowed(p, new_mask, flags);
>
> -       if (p->flags & PF_KTHREAD) {
> -               /*
> -                * For kernel threads that do indeed end up on online &&
> -                * !active we want to ensure they are strict per-CPU threads.
> -                */
> -               WARN_ON(cpumask_intersects(new_mask, cpu_online_mask) &&
> -                       !cpumask_intersects(new_mask, cpu_active_mask) &&
> -                       p->nr_cpus_allowed != 1);
> -       }
> -
>         return affine_move_task(rq, p, &rf, dest_cpu, flags);
>
>  out:
> @@ -7519,6 +7511,12 @@ int sched_cpu_deactivate(unsigned int cp
>          */
>         synchronize_rcu();
>
> +       /*
> +        * From this point forward, this CPU will refuse to run any task that
> +        * is not: migrate_disable() or KTHREAD_IS_PER_CPU, and will actively
> +        * push those tasks away until this gets cleared, see
> +        * sched_cpu_dying().
> +        */
>         balance_push_set(cpu, true);
>
>         rq_lock_irqsave(rq, &rf);
>
>
Peter Zijlstra Jan. 16, 2021, 3:19 p.m. UTC | #2
On Sat, Jan 16, 2021 at 10:39:03PM +0800, Lai Jiangshan wrote:
> On Sat, Jan 16, 2021 at 7:43 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Now that we have KTHREAD_IS_PER_CPU to denote the critical per-cpu
> > tasks to retain during CPU offline, we can relax the warning in
> > set_cpus_allowed_ptr(). Any spurious kthread that wants to get on at
> > the last minute will get pushed off before it can run.
> >
> > While during CPU online there is no harm, and actual benefit, to
> > allowing kthreads back on early, it simplifies hotplug code.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Thanks!
> 
> Relaxing set_cpus_allowed_ptr() was also one of the choices I listed,
> which can really simplify hotplug code in the workqueue and may be
> other hotplug code.

Indeed you did. Having that KTHREAD_IS_PER_CPU for the offline side of
things made it possible to relax (as per the Changelog above).

> Reviewed-by: Lai jiangshan <jiangshanlai@gmail.com>

Thanks!

Patch
diff mbox series

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2342,7 +2342,9 @@  static int __set_cpus_allowed_ptr(struct
 
 	if (p->flags & PF_KTHREAD || is_migration_disabled(p)) {
 		/*
-		 * Kernel threads are allowed on online && !active CPUs.
+		 * Kernel threads are allowed on online && !active CPUs,
+		 * however, during cpu-hot-unplug, even these might get pushed
+		 * away if not KTHREAD_IS_PER_CPU.
 		 *
 		 * Specifically, migration_disabled() tasks must not fail the
 		 * cpumask_any_and_distribute() pick below, esp. so on
@@ -2386,16 +2388,6 @@  static int __set_cpus_allowed_ptr(struct
 
 	__do_set_cpus_allowed(p, new_mask, flags);
 
-	if (p->flags & PF_KTHREAD) {
-		/*
-		 * For kernel threads that do indeed end up on online &&
-		 * !active we want to ensure they are strict per-CPU threads.
-		 */
-		WARN_ON(cpumask_intersects(new_mask, cpu_online_mask) &&
-			!cpumask_intersects(new_mask, cpu_active_mask) &&
-			p->nr_cpus_allowed != 1);
-	}
-
 	return affine_move_task(rq, p, &rf, dest_cpu, flags);
 
 out:
@@ -7519,6 +7511,12 @@  int sched_cpu_deactivate(unsigned int cp
 	 */
 	synchronize_rcu();
 
+	/*
+	 * From this point forward, this CPU will refuse to run any task that
+	 * is not: migrate_disable() or KTHREAD_IS_PER_CPU, and will actively
+	 * push those tasks away until this gets cleared, see
+	 * sched_cpu_dying().
+	 */
 	balance_push_set(cpu, true);
 
 	rq_lock_irqsave(rq, &rf);