Developers split over split-lock detection
Split locks are, in essence, a type of misaligned memory access — something that the x86 architecture has always been relatively willing to forgive. But there is a difference: while a normal unaligned operation will slow down the process performing that operation, split locks will slow down the entire system. The 1,000-cycle delay may be particularly painful on realtime systems, but split locks can be used as an effective denial-of-service attack on any system. There is little disagreement among developers that their use should not be allowed on most systems.
Recent Intel processors can be configured to execute a trap when a split lock is attempted, allowing the operating system to decide whether to allow the operation to continue. Fenghua Yu first posted a patch set enabling this trap in May 2018; LWN covered this work one year later. While many things have changed in this patch set, the basic idea has remained constant: when this feature is enabled, user-space processes that attempt a split lock will receive a SIGBUS signal. What happens when split locks are created in other contexts has varied over time; in the version-10 patch set posted in late November, split locks in the kernel or system firmware will cause a kernel panic.
This severe response should result in problems being fixed quickly; Yu cites a couple of kernel fixes for split locks detected by this work in the patch posting. That will only happen, though, if this feature is enabled on systems where the code tries to create a split lock, but that may not happen as often as developers would like:
Disabling the feature by default guarantees that it will not be widely
used; that has led to some complaints. Ingo Molnar asserted that
"for this feature to be useful it must be default-enabled
",
and Peter Zijlstra said:
Zijlstra is particularly concerned about split locks created by code at the
firmware level, something he sees as being likely: "from long and
painful experience we all know that if a BIOS can be wrong, it will
be
". Enabling split-lock detection by default will hopefully cause
firmware-created split locks to be fixed. Otherwise, he fears, those split
locks will lurk in the firmware indefinitely, emerging occasionally to burn
users who enable split-lock detection in the hope of finding problems in
their own code.
Forcing problems to be fixed by enabling split-lock detection by default has some obvious appeal. But the reasoning behind leaving it disabled also makes some sense: killing processes that create split locks is an ABI change that may create problems for users. As Tony Luck put it:
#include <linus/all-caps-rant-about-backwards-compatability.h>
and the patches being reverted.
Zijlstra is not worried, though; he feels that the kernel issues have mostly been fixed and that problems in user-space code will be rare because other architectures have never allowed split locks. For those who are worried, though, he posted a follow-up patch allowing split-lock detection to be controlled at boot time and adding a "warn-only" mode that doesn't actually kill any processes.
In that patch set, he noted that "it requires we get the kernel and
firmware clean
", and said that fixing up the kernel should be
"pretty simple
". But it turns out to be perhaps not quite
that simple after all. In particular, there is the problem pointed
out by David Laight: the kernel's atomic
bitmask functions can easily create split-lock operations. The core
problem here is that the type of a bitmask is defined as unsigned
long, but it's natural for developers to use a simple int
instead. In such cases, the creation of misaligned accesses is easy, and
those accesses may occasionally span a cache-line boundary and lead to
split locks.
Opinions on how to solve this problem globally vary. Yu posted a
complex series of cases meant to make atomic bit operations work for
almost all usage patterns, but that strikes some as too much complexity;
Zijlstra said
simply that this solution is "never going to happen
". An
alternative, suggested
by Andy Lutomirski, is to change the atomic bit operations to work on
32-bit values. That would, obviously, limit the number of bits that could
be manipulated to 32. Zijlstra noted
that some architectures (alpha, ia64) already implement atomic bit
operations that way, so it may well be that 32 bits is all that the
kernel needs.
There is one other "wrinkle", according to Sean Christopherson, getting in the way of merging split-lock detection: the processor bit that controls split-lock detection affects the entire core on which it's set, not just the specific CPU that sets it. So toggling split-lock detection will affect all hyperthreaded siblings together. This particular wrinkle was only discovered in September, after the split-lock patch set had already been through nine revisions, leaving the development community less than fully impressed. But now that this behavior is known, it must be dealt with in the kernel.
If split-lock detection is enabled globally, there is no problem. But if there is a desire to enable and disable it at specific times, things may well not work as expected. Things get particularly difficult when virtualization is involved; guest systems may differ with each other — or with the host — about whether split-lock detection should be enabled. Potential solutions to this problem include disallowing control of split-lock detection in guests (the current solution) or only allowing it when hyperthreading is disabled. Nobody has yet suggested using core scheduling to ensure that all processes running on a given core are in agreement about split-lock detection, but it's only a matter of time.
All of this adds up to a useful feature that is apparently not yet ready
for prime time. The question at this point is whether it should be merged
soon and improved in place, or whether it needs more work out of tree.
Perhaps both things could happen, since 5.6 is the earliest time it could
be pulled into the mainline at this point. Split-lock detection exists to
eliminate unwanted delays, but it still seems subject to some delays
itself.
| Index entries for this article | |
|---|---|
| Kernel | Architectures/x86 |