- Notifications
You must be signed in to change notification settings - Fork480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Do not disable interrupts when building topology#435
base:master
Are you sure you want to change the base?
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!
|
8c0fcff tob08ecacCompareI 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.