workflows.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: cl@gentwo.org
Cc: Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	 Jonathan Corbet <corbet@lwn.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Yang Shi <shy828301@gmail.com>,
	Huang Shijie <shijie@os.amperecomputing.com>,
	 kasan-dev@googlegroups.com, workflows@vger.kernel.org,
	 linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Christoph Lameter <cl@linux.com>
Subject: Re: [PATCH] KFENCE: Clarify that sample allocations are not following NUMA or memory policies
Date: Fri, 24 Jan 2025 00:44:19 +0100	[thread overview]
Message-ID: <CANpmjNM04i3bNYJXYP8aEKy_-o=MTiW-eBEb9NmzpHoaTxwQTg@mail.gmail.com> (raw)
In-Reply-To: <20250123-kfence_doc_update-v1-1-9aa8e94b3d0b@gentwo.org>

On Thu, 23 Jan 2025 at 23:44, Christoph Lameter via B4 Relay
<devnull+cl.gentwo.org@kernel.org> wrote:
>
> From: Christoph Lameter <cl@linux.com>
>
> 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@linux.com>
> ---
>  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.

  parent reply	other threads:[~2025-01-23 23:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-23 22:44 Christoph Lameter via B4 Relay
2025-01-23 23:33 ` Yang Shi
2025-01-23 23:44 ` Marco Elver [this message]
2025-01-24  8:13 ` Vlastimil Babka
2025-01-24  8:37   ` Marco Elver
2025-01-24  8:42     ` Vlastimil Babka
2025-01-24 12:03       ` Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANpmjNM04i3bNYJXYP8aEKy_-o=MTiW-eBEb9NmzpHoaTxwQTg@mail.gmail.com' \
    --to=elver@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=cl@linux.com \
    --cc=corbet@lwn.net \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shijie@os.amperecomputing.com \
    --cc=shy828301@gmail.com \
    --cc=workflows@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox