Do not disable interrupts when building topology #435
Conversation
|
This commit needs to be updated for conflicts, but, if someone could let me know if I'm on the right track, I could continue or not continue with this patch. Thank you. |
|
BTW, if it's interesting what the output looks like on my 2019 MacBook Pro (Coffee Lake/i5-8257U), I've been testing by doing runs of pcm & pcm-core measuring DTLB misses that cause a page table walk (the event codes are taken from /usr/share/kpep/). However, real world benchmarking is still something I'm making progress on, and macOS doesn't seem to expose a way to set CPU affinity, although it does support L2 cache affinity for separate tasks, so that they are scheduled on the same CPU. Thanks!
|
nealsid
force-pushed
the
get-topo-with-interrupts-enabled
branch
from
8c0fcff to
b08ecac
Compare
|
I am still trying to find a reviewer for your patch |
Appreciate it! |
|
If it's helpful, it wasn't clear to me why executing CPUID would require interrupts to be disabled (or even need to be in the kernel, but then I realized it needs to execute on all CPUs to build the topology), because, AFAIUI, interrupts are generally disabled when taking a lock that might be required by an interrupt handler. Since there are variants of the mp_rendezvous function that disable or do not disable interrupts, I also assumed that the barrier functionality it provides does not depend on disabling interrupts. Thanks! |
|
I started digging into this some more, and mp_rendezvous, internally, will disable interrupts on non "master" CPUs (the terminology is from the XNU source and I believe it refers to the startup CPU) while enqueueing the function to be run on non-local CPUs, in order to prevent topology changes while scheduling the function: https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/osfmk/i386/mp.c#L909 So the references to disabling or enabling interrupts in mp_rendezvous/mp_rendezvous_no_intr are for while the function itself is running, and it will not cause a race condition with topology changes. |
Currently, interrupts are disabled during part of the process for building CPU topology on macOS, but it is not clear to me why this is necessary, so this patch uses
mp_rendezvousto run CPUID on all CPUs instead ofmp_rendezvous_no_intrs.