On Sat, Nov 28, 2020 at 8:02 AM Nicholas Piggin <npiggin@xxxxxxxxx> wrote: > > On big systems, the mm refcount can become highly contented when doing > a lot of context switching with threaded applications (particularly > switching between the idle thread and an application thread). > > Abandoning lazy tlb slows switching down quite a bit in the important > user->idle->user cases, so so instead implement a non-refcounted scheme > that causes __mmdrop() to IPI all CPUs in the mm_cpumask and shoot down > any remaining lazy ones. > > Shootdown IPIs are some concern, but they have not been observed to be > a big problem with this scheme (the powerpc implementation generated > 314 additional interrupts on a 144 CPU system during a kernel compile). > There are a number of strategies that could be employed to reduce IPIs > if they turn out to be a problem for some workload. I'm still wondering whether we can do even better. The IPIs you're doing aren't really necessary -- we don't fundamentally need to free the pagetables immediately when all non-lazy users are done with them (and current kernels don't) -- what we need to do is to synchronize all the bookkeeping. So, with adequate locking (famous last words), a couple of alternative schemes ought to be possible. a) Instead of sending an IPI, increment mm_count on behalf of the remote CPU and do something to make sure that the remote CPU knows we did this on its behalf. Then free the mm when mm_count hits zero. b) Treat mm_cpumask as part of the refcount. Add one to mm_count when an mm is created. Once mm_users hits zero, whoever clears the last bit in mm_cpumask is responsible for decrementing a single reference from mm_count, and whoever sets it to zero frees the mm. Version (b) seems fairly straightforward to implement -- add RCU protection and a atomic_t special_ref_cleared (initially 0) to struct mm_struct itself. After anyone clears a bit to mm_cpumask (which is already a barrier), they read mm_users. If it's zero, then they scan mm_cpumask and see if it's empty. If it is, they atomically swap special_ref_cleared to 1. If it was zero before the swap, they do mmdrop(). I can imagine some tweaks that could make this a big faster, at least in the limit of a huge number of CPUs. Version (a) seems a bit harder to reason about. Maybe it could be done like this. Add a percpu variable mm_with_extra_count. This variable can be NULL, but it can also be an mm that has an extra reference on behalf of the cpu in question. __mmput scans mm_cpumask and, for each cpu in the mask, mmgrabs the mm and cmpxchgs that cpu's mm_with_extra_count from NULL to mm. If it succeeds, then we win. If it fails, further thought is required, and maybe we have to send an IPI, although maybe some other cleverness is possible. Any time a CPU switches mms, it does atomic swaps mm_with_extra_count to NULL and mmdrops whatever the mm was. (Maybe it needs to check the mm isn't equal to the new mm, although it would be quite bizarre for this to happen.) Other than these mmgrab and mmdrop calls, the mm switching code doesn't mmgrab or mmdrop at all. Version (a) seems like it could have excellent performance. *However*, I think we should consider whether we want to do something even bigger first. Even with any of these changes, we still need to maintain mm_cpumask(), and that itself can be a scalability problem. I wonder if we can solve this problem too. Perhaps the switch_mm() paths could only ever set mm_cpumask bits, and anyone who would send an IPI because a bit is set in mm_cpumask would first check some percpu variable (cpu_rq(cpu)->something? an entirely new variable) to see if the bit in mm_cpumask is spurious. Or perhaps mm_cpumask could be split up across multiple cachelines, one per node. We should keep the recent lessons from Apple in mind, though: x86 is a dinosaur. The future of atomics is going to look a lot more like ARM's LSE than x86's rather anemic set. This means that mm_cpumask operations won't need to be full barriers forever, and we might not want to take the implied full barriers in set_bit() and clear_bit() for granted. --Andy
