lkml.org 
[lkml]   [2021]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 7/8] sched: Fix CPU hotplug / tighten is_per_cpu_kthread()
Date
On 16/01/21 12:30, Peter Zijlstra wrote:
> @@ -1796,13 +1796,28 @@ static inline bool rq_has_pinned_tasks(s
> */
> static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
> {
> + /* When not in the task's cpumask, no point in looking further. */
> if (!cpumask_test_cpu(cpu, p->cpus_ptr))
> return false;
>
> - if (is_per_cpu_kthread(p) || is_migration_disabled(p))
> + /* migrate_disabled() must be allowed to finish. */
> + if (is_migration_disabled(p))
> return cpu_online(cpu);
>
> - return cpu_active(cpu);
> + /* Non kernel threads are not allowed during either online or offline. */
> + if (!(p->flags & PF_KTHREAD))
> + return cpu_active(cpu);
> +
> + /* KTHREAD_IS_PER_CPU is always allowed. */
> + if (kthread_is_per_cpu(p))
> + return cpu_online(cpu);
> +
> + /* Regular kernel threads don't get to stay during offline. */
> + if (cpu_rq(cpu)->balance_callback == &balance_push_callback)
> + return cpu_active(cpu);

is_cpu_allowed(, cpu) isn't guaranteed to have cpu_rq(cpu)'s rq_lock
held, so this can race with balance_push_set(, true). This shouldn't
matter under normal circumstances as we'll have sched_cpu_wait_empty()
further down the line.

This might get ugly with the rollback faff - this is jumping the gun a
bit, but that's something we'll have to address, and I think what I'm
concerned about is close to what you mentioned in

http://lore.kernel.org/r/YAM1t2Qzr7Rib3bN@hirez.programming.kicks-ass.net

Here's what I'm thinking of:

_cpu_up() ttwu()
select_task_rq()
is_cpu_allowed()
rq->balance_callback != balance_push_callback
smpboot_unpark_threads() // FAIL
(now going down, set push here)
sched_cpu_wait_empty()
... ttwu_queue()
sched_cpu_dying()
*ARGH*

I've written some horrors on top of this series here:

https://gitlab.arm.com/linux-arm/linux-vs/-/commits/mainline/migrate_disable/stragglers/

Also, my TX2 is again in need of CPR, so in the meantime I'm running
tests on a (much) smaller machine...

> +
> + /* But are allowed during online. */
> + return cpu_online(cpu);
> }

\
 
 \ /
  Last update: 2021-01-17 17:59    [W:0.163 / U:0.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site