All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: syzbot <syzbot+6835d5c11145e4f77057@syzkaller.appspotmail.com>,
	linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@kernel.org>,
	Radu Rendec <radu@rendec.net>, Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>
Subject: Re: [RFC PATCH v2] sched: irq: cpu-hotplug race vs set_cpus_allowed_ptr()
Date: Mon, 12 Oct 2026 16:30:47 +0200	[thread overview]
Message-ID: <20261012143047.GV2009045@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20261012103946.aOSAmrRe@linutronix.de>

On Mon, Oct 12, 2026 at 12:39:46PM +0200, Sebastian Andrzej Siewior wrote:
> syzbot reported a race of assigning a CPU affinity during CPU hotplug
> operation.
> After the affinity of an IRQ has been changed, the IRQ-core sets
> IRQTF_AFFINITY and wakes the relevant interrupt threads which need to
> adjust their affinity mask. The thread will then invoke
> set_cpus_allowed_ptr() to update the mask. Based on the new mask a CPU
> is chosen on which the thread should run.
> 
> It is verified that this CPU is online however there is no guarantee
> that this CPU remains online while affine_move_task() is moving _this_
> task. Since the current task requests the migration the stopper/
> migration_cpu_stop() is involved at which point the task pauses for a
> while. If the CPU goes offline (or is no longer cpu_online_mask) then
> __migrate_task() (due to is_cpu_allowed() reject) will return the rq of
> the CPU on which the task is currently running (which does not match
> it's task_struct::cpus_mask).
> 
> The aftermath:
> The IRQ-thread's CPU and task's cpus_mask do not match. A
> migrate_disable()-> schedule() will change task_struct::cpus_ptr
> ensuring that the following migrate_enable() will update the task to the
> requested affinity mask. At this point, affine_move_task() expects
> task_struct::migration_pending set but it is NULL because no one
> requested an affinity change while the task was in migrate-disable
> section.



> I see two ways of fixing this:
> - Holding the cpus_read_lock while set_cpus_allowed_ptr() is invoked.
>   This ensure that the CPU remains in the cpu_online_mask while the
>   migration task moving the task over. Should the mask be already
>   invalid, then it is rejected otherwise the operation completes.
> 
>   sashiko complains about deadlock of cpus_write_lock() in the CPU down
>   path and calling irq_migrate_all_off_this_cpu() vs
>   irq_thread_check_affinity() doing cpus_read_lock().
>   As far as I see, there is no deadlock because the mask is updated, the
>   tread is woken, the tread blocks with cpus_read_lock() until the CPU is
>   down.
>   There is nothing that waits for the interrupt thread to complete,
>   there is no synchronize_irq() invocation which would be required for
>   the deadlock to happen.

But any other set_cpus_allowed_ptr() call in the kernel can suffer the
same problem, no? Why is the irq thing 'special'?


> - Should __migrate_task() fail to return the requested rq make sure its
>   CPU is part task_struct::cpus_mask. This ensures that the current CPU
>   is still part of mask avoiding a possible push by migrate_enable().
>   sched_class::set_cpus_allowed did not see this mask. It might be a bit
>   inconsistent and feels a bit like select_fallback_rq() without the
>   printk.
> 
>   Sashiko complains:
> | If __migrate_task() fails (e.g., due to a concurrent CPU offline event), this
> | fallback code forcibly modifies p->cpus_mask with cpumask_set_cpu(). This
> | skips the required p->sched_class->set_cpus_allowed() callback and fails to
> | update p->nr_cpus_allowed.
> | Can this bypass global EDF real-time invariants for SCHED_DEADLINE (as
> | set_cpus_allowed_dl() is skipped), and allow unprivileged tasks to escape
> | affinity boundaries, giving them access to CPUs outside their cpusets?
> 
>   We did do_set_cpus_allowed() for CPU X and can't migrate there now. We
>   also don't know if the invocation was successful.
>   
>   Looking at this, it is not possible acquire the CPU-hotplug lock here
>   and not every caller holds it. This seems to be limited to
>   migrate-disable. Should this happen without the lock, then during the
>   wake-up the scheduler will just print "process … no longer affine to
>   cpu" pick a CPU and be done with it.
>   Maybe set the bit followed by do_set_cpus_allowed()?
> 
> Both changes are implemented to illustrate, one is enough.
> I can reproduce this back on v6.8, therefore I assume we have this since
> day #1 of migrate-disable.

I am struggling to follow along. The actual fail scenario is a
combination of hotplug *and* migrate_disable()? Can we get a picture?

A CPU must not be able to go offline while there are migrate_disable()
tasks on it. Are you saying the migrate_disable() happens after hotplug
already marks the CPU offline?

The thinking was that either:

 - task is asleep, and ttwu()/select_task_rq() picks a fallback
 - task is running, and __balance_push_cpu_stop() picks a fallback


There is a TLA+ model of all this somewhere; Valentin, do you remember?
That would have to be extended at the very least.



  parent reply	other threads:[~2026-10-12 14:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-12 10:39 [RFC PATCH v2] sched: irq: cpu-hotplug race vs set_cpus_allowed_ptr() Sebastian Andrzej Siewior
2026-10-12 14:19 ` Juri Lelli
2026-10-12 14:30 ` Peter Zijlstra [this message]
2026-10-13  6:46   ` Sebastian Andrzej Siewior
2026-10-13 13:43     ` Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261012143047.GV2009045@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bigeasy@linutronix.de \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=radu@rendec.net \
    --cc=rostedt@goodmis.org \
    --cc=syzbot+6835d5c11145e4f77057@syzkaller.appspotmail.com \
    --cc=tglx@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.