Messages in this thread | | | Date | Thu, 21 Nov 2019 09:34:44 -0800 | From | "Luck, Tony" <> | Subject | Re: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by kernel parameter |
| |
On Thu, Nov 21, 2019 at 06:12:14PM +0100, Ingo Molnar wrote: > > * Peter Zijlstra <peterz@infradead.org> wrote: > > > On Thu, Nov 21, 2019 at 07:04:44AM +0100, Ingo Molnar wrote: > > > * Fenghua Yu <fenghua.yu@intel.com> wrote: > > > > > > + split_lock_detect > > > > + [X86] Enable split lock detection > > > > + This is a real time or debugging feature. When enabled > > > > + (and if hardware support is present), atomic > > > > + instructions that access data across cache line > > > > + boundaries will result in an alignment check exception. > > > > + When triggered in applications the kernel will send > > > > + SIGBUS. The kernel will panic for a split lock in > > > > + OS code. > > > > > > It would be really nice to be able to enable/disable this runtime as > > > well, has this been raised before, and what was the conclusion? > > > > It has, previous versions had that. Somehow a lot of things went missing > > and we're back to a broken neutered useless mess. > > > > The problem appears to be that due to hardware design the feature cannot > > be virtualized, and instead of then disabling it when a VM runs/exists > > they just threw in the towel and went back to useless mode.. :-( > > > > This feature MUST be default enabled, otherwise everything will > > be/remain broken and we'll end up in the situation where you can't use > > it even if you wanted to. > > Agreed. > > > And I can't be arsed to look it up, but we've been making this very > > same argument since very early (possible the very first) version. > > Yeah, I now have a distinct deja vu...
You'll notice that we are at version 10 ... lots of things have been tried in previous versions. This new version is to get the core functionality in, so we can build fancier features later. Painful experience has shown that trying to do this all at once just leads to churn with no progress.
Enabling by default at this point would result in a flurry of complaints about applications being killed and kernels panicing. That would be followed by:
#include <linus/all-caps-rant-about-backwards-compatability.h>
and the patches being reverted.
This version can serve a very useful purpose. CI systems with h/w that supports split lock can enable it and begin the process of finding and fixing the remaining kernel issues. Especially helpful if they run randconfig and fuzzers.
We'd also find out which libraries and applications currently use split locks.
Real-time folks that have identified split lock as a fatal (don't meet their deadlines issue) could also enable it as is (because it is better to crash the kernel and have the laser be powered down than to keep firing long past the point it should have stopped).
Any developer with concerns about their BIOS using split locks can also enable using this patch and begin testing today.
I'm totally on board with follow up patches providing extra features like:
A way to enable/disable at run time.
Providing a way to allow but rate limit applications that cause split locks.
Figuring out something useful to do with virtualization.
Those are all good things to have - but we won't get *any* of them if we wait until *all* have them have been perfected.
<soapbox> So let's just take the first step now and solve world hunger tomorrow. </soapbox>
-Tony
| |