On Thu, Sep 30 2021 at 15:01, Andy Lutomirski wrote:
On Thu, Sep 30, 2021, at 12:29 PM, Thomas Gleixner wrote:
But even with that we still need to keep track of the armed ones per CPU so we can handle CPU hotunplug correctly. Sigh...
I don’t think any real work is needed. We will only ever have armed UPIDs (with notification interrupts enabled) for running tasks, and hot-unplugged CPUs don’t have running tasks.
That's not the problem. The problem is the wait for uintr case where the task is obviously not running:
CPU 1 upid = T1->upid; upid->vector = UINTR_WAIT_VECTOR; upid->ndst = local_apic_id(); ... do { .... schedule(); }
CPU 0 unplug CPU 1
SENDUPI(index) // Hardware does: tblentry = &ttable[index]; upid = tblentry->upid; upid->pir |= tblentry->uv; send_IPI(upid->vector, upid->ndst);
So SENDUPI will send the IPI to the APIC ID provided by T1->upid.ndst which points to the offlined CPU 1 and therefore is obviously going to /dev/null. IOW, lost wakeup...
We do need a way to drain pending IPIs before we offline a CPU, but that’s a separate problem and may be unsolvable for all I know. Is there a magic APIC operation to wait until all initiated IPIs targeting the local CPU arrive? I guess we can also just mask the notification vector so that it won’t crash us if we get a stale IPI after going offline.
All of this is solved already otherwise CPU hot unplug would explode in your face every time. The software IPI send side is carefully synchronized vs. hotplug (at least in theory). May I ask you politely to make yourself familiar with all that before touting "We do need..." based on random assumptions?
The above SENDUIPI vs. CPU hotplug scenario is the same problem as we have with regular device interrupts which are targeted at an outgoing CPU. We have magic mechanisms in place to handle that to the extent possible, but due to the insanity of X86 interrupt handling mechanics that still leaves a very tiny hole which might cause a lost and subsequently stale interrupt. Nothing we can fix in software.
So on CPU offline the hotplug code walks through all device interrupts and checks whether they are targeted at the outgoing CPU. If so they are rerouted to an online CPU with lots of care to make the possible race window as small as it gets. That's nowadays only a problem on systems where interrupt remapping is not available or disabled via commandline.
For tasks which just have the user interrupt armed there is no problem because SENDUPI modifies UPID->PIR which is reevaluated when the task which got migrated to an online CPU is going back to user space.
The uintr_wait() syscall creates the very same problem as we have with device interrupts. Which means we need to make that wait thing:
upid = T1->upid; upid->vector = UINTR_WAIT_VECTOR; upid->ndst = local_apic_id(); list_add(this_cpu_ptr(pcp_uintrs), upid->pcp_uintr); ... do { .... schedule(); } list_del_init(upid->pcp_uintr);
and the hotplug code do:
for_each_entry_safe(upid, this_cpu_ptr(pcp_uintrs), ...) { list_del(upid->pcp_uintr); upid->ndst = apic_id_of_random_online_cpu(); if (do_magic_checks_whether_ipi_is_pending()) send_ipi(upid->vector, upid->ndst); }
See?
We could map that to the interrupt subsystem by creating a virtual interrupt domain for this, but that would make uintr_wait() look like this:
irq = uintr_alloc_irq(); request_irq(irq, ......); upid = T1->upid; upid->vector = UINTR_WAIT_VECTOR; upid->ndst = local_apic_id(); list_add(this_cpu_ptr(pcp_uintrs), upid->pcp_uintr); ... do { .... schedule(); } list_del_init(upid->pcp_uintr); free_irq(irq);
But the benefit of that is dubious as it creates overhead on both sides of the sleep and the only real purpose of the irq request would be to handle CPU hotunplug without the above per CPU list mechanics.
Welcome to my wonderful world!
Thanks,
tglx