pcm icon indicating copy to clipboard operation
pcm copied to clipboard

Do not disable interrupts when building topology

Open nealsid opened this issue 2 years ago • 6 comments

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_rendezvous to run CPUID on all CPUs instead of mp_rendezvous_no_intrs.

nealsid avatar Aug 31 '22 08:08 nealsid

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.

nealsid avatar Sep 08 '22 19:09 nealsid

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!

Screenshot 2022-09-09 at 1 13 19 PM

nealsid avatar Sep 09 '22 20:09 nealsid

I am still trying to find a reviewer for your patch

opcm avatar Sep 12 '22 06:09 opcm

I am still trying to find a reviewer for your patch

Appreciate it!

nealsid avatar Sep 12 '22 19:09 nealsid

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!

nealsid avatar Sep 14 '22 03:09 nealsid

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 https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/osfmk/i386/mp.c#L1323

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.

nealsid avatar Sep 14 '22 03:09 nealsid