On Thu, 23 Jan 2025 at 23:44, Christoph Lameter via B4 Relay <devnull+cl.gentwo.org@xxxxxxxxxx> wrote: > > From: Christoph Lameter <cl@xxxxxxxxx> > > KFENCE manages its own pools and redirects regular memory allocations > to those pools in a sporadic way. The usual memory allocator features > like NUMA, memory policies and pfmemalloc are not supported. > This means that one gets surprising object placement with KFENCE that > may impact performance on some NUMA systems. > > Update the description and make KFENCE depend on VM debugging > having been enabled. While the documentation updates are fine with me, the Kconfig change seems overly drastic. What's the motivation? CONFIG_KFENCE is not enabled by default, and if there's a problem users are free to either not select it in the first place, or if you cannot unset CONFIG_KFENCE because you have a prebuilt kernel, set 'kfence.sample_interval=0' in the kernel cmdline. More commentary below. > Signed-off-by: Christoph Lameter <cl@xxxxxxxxx> > --- > Documentation/dev-tools/kfence.rst | 4 +++- > lib/Kconfig.kfence | 10 ++++++---- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/Documentation/dev-tools/kfence.rst b/Documentation/dev-tools/kfence.rst > index 541899353865..27150780d6f5 100644 > --- a/Documentation/dev-tools/kfence.rst > +++ b/Documentation/dev-tools/kfence.rst > @@ -8,7 +8,9 @@ Kernel Electric-Fence (KFENCE) is a low-overhead sampling-based memory safety > error detector. KFENCE detects heap out-of-bounds access, use-after-free, and > invalid-free errors. > > -KFENCE is designed to be enabled in production kernels, and has near zero > +KFENCE is designed to be low overhead but does not implememnt the typical s/implememnt/implement/ > +memory allocation features for its samples like memory policies, NUMA and > +management of emergency memory pools. It has near zero > performance overhead. Compared to KASAN, KFENCE trades performance for > precision. The main motivation behind KFENCE's design, is that with enough > total uptime KFENCE will detect bugs in code paths not typically exercised by > diff --git a/lib/Kconfig.kfence b/lib/Kconfig.kfence > index 6fbbebec683a..48d2a6a1be08 100644 > --- a/lib/Kconfig.kfence > +++ b/lib/Kconfig.kfence > @@ -5,14 +5,14 @@ config HAVE_ARCH_KFENCE > > menuconfig KFENCE > bool "KFENCE: low-overhead sampling-based memory safety error detector" > - depends on HAVE_ARCH_KFENCE > + depends on HAVE_ARCH_KFENCE && DEBUG_VM This is not going to work. There are plenty deployments of KFENCE in kernels that do not enable DEBUG_VM, and this will silently disable KFENCE once those kernels upgrade. And enabling DEBUG_VM is not what anybody wants, because enabling DEBUG_VM adds features significantly more expensive than KFENCE, even if disabled they pull in code and increase .text size. Nack with the dependency on DEBUG_VM. The documentation change is fine. > select STACKTRACE > select IRQ_WORK > help > KFENCE is a low-overhead sampling-based detector of heap out-of-bounds > access, use-after-free, and invalid-free errors. KFENCE is designed > - to have negligible cost to permit enabling it in production > - environments. > + to have negligible cost. KFENCE does not support NUMA features > + and other memory allocator features for it sample allocations. s/sample/samples/ > See <file:Documentation/dev-tools/kfence.rst> for more details. > > @@ -21,7 +21,9 @@ menuconfig KFENCE > detect, albeit at very different performance profiles. If you can > afford to use KASAN, continue using KASAN, for example in test > environments. If your kernel targets production use, and cannot > - enable KASAN due to its cost, consider using KFENCE. > + enable KASAN due to its cost and you are not using NUMA and have > + no use of the memory reserve logic of the memory allocators, > + consider using KFENCE. That's just repetition from above, and I think the point here is just that if you run tests but can't use KASAN, consider KFENCE. In those cases, users typically would use much higher sampling rates that _will_ have somewhat noticeable performance impact. Thanks.
