Message-ID: <CANpmjNMmiXjifpc9LdCVi5jzzKU3sgb0iJn7P7TMFMNqDH7TbA@mail.gmail.com>
Date: Thu, 18 Dec 2025 11:23:33 +0100
From: Marco Elver <elver@...gle.com>
To: yuanlinyu <yuanlinyu@...or.com>
Cc: Alexander Potapenko <glider@...gle.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Huacai Chen <chenhuacai@...nel.org>, 
	WANG Xuerui <kernel@...0n.name>, 
	"kasan-dev@...glegroups.com" <kasan-dev@...glegroups.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, 
	"loongarch@...ts.linux.dev" <loongarch@...ts.linux.dev>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] kfence: allow change number of object by early parameter

On Thu, 18 Dec 2025 at 11:18, yuanlinyu <yuanlinyu@...or.com> wrote:
>
> > From: Marco Elver <elver@...gle.com>
> > Sent: Thursday, December 18, 2025 4:57 PM
> > To: yuanlinyu <yuanlinyu@...or.com>
> > Cc: Alexander Potapenko <glider@...gle.com>; Dmitry Vyukov
> > <dvyukov@...gle.com>; Andrew Morton <akpm@...ux-foundation.org>;
> > Huacai Chen <chenhuacai@...nel.org>; WANG Xuerui <kernel@...0n.name>;
> > kasan-dev@...glegroups.com; linux-mm@...ck.org; loongarch@...ts.linux.dev;
> > linux-kernel@...r.kernel.org
> > Subject: Re: [PATCH v2 2/2] kfence: allow change number of object by early
> > parameter
> >
> > On Thu, Dec 18, 2025 at 02:39PM +0800, yuan linyu wrote:
> > > when want to change the kfence pool size, currently it is not easy and
> > > need to compile kernel.
> > >
> > > Add an early boot parameter kfence.num_objects to allow change kfence
> > > objects number and allow increate total pool to provide high failure
> > > rate.
> > >
> > > Signed-off-by: yuan linyu <yuanlinyu@...or.com>
> > > ---
> > >  include/linux/kfence.h  |   5 +-
> > >  mm/kfence/core.c        | 122
> > +++++++++++++++++++++++++++++-----------
> > >  mm/kfence/kfence.h      |   4 +-
> > >  mm/kfence/kfence_test.c |   2 +-
> > >  4 files changed, 96 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/include/linux/kfence.h b/include/linux/kfence.h
> > > index 0ad1ddbb8b99..920bcd5649fa 100644
> > > --- a/include/linux/kfence.h
> > > +++ b/include/linux/kfence.h
> > > @@ -24,7 +24,10 @@ extern unsigned long kfence_sample_interval;
> > >   * address to metadata indices; effectively, the very first page serves as an
> > >   * extended guard page, but otherwise has no special purpose.
> > >   */
> > > -#define KFENCE_POOL_SIZE ((CONFIG_KFENCE_NUM_OBJECTS + 1) * 2 *
> > PAGE_SIZE)
> > > +extern unsigned int __kfence_pool_size;
> > > +#define KFENCE_POOL_SIZE (__kfence_pool_size)
> > > +extern unsigned int __kfence_num_objects;
> > > +#define KFENCE_NUM_OBJECTS (__kfence_num_objects)
> > >  extern char *__kfence_pool;
> > >
> >
> > You have ignored the comment below in this file:
> >
> >       /**
> >        * is_kfence_address() - check if an address belongs to KFENCE pool
> >        * @addr: address to check
> >        *
> >       [...]
> >        * Note: This function may be used in fast-paths, and is performance
> > critical.
> >        * Future changes should take this into account; for instance, we want to
> > avoid
> >    >>  * introducing another load and therefore need to keep
> > KFENCE_POOL_SIZE a
> >    >>  * constant (until immediate patching support is added to the kernel).
> >        */
> >       static __always_inline bool is_kfence_address(const void *addr)
> >       {
> >               /*
> >                * The __kfence_pool != NULL check is required to deal with the case
> >                * where __kfence_pool == NULL && addr < KFENCE_POOL_SIZE.
> > Keep it in
> >                * the slow-path after the range-check!
> >                */
> >               return unlikely((unsigned long)((char *)addr - __kfence_pool) <
> > KFENCE_POOL_SIZE && __kfence_pool);
> >       }
>
> Do you mean performance critical by access global data ?
> It already access __kfence_pool global data.
> Add one more global data acceptable here ?
>
> Other place may access global data indeed ?

is_kfence_address() is used in the slub fast path, and another load is
one more instruction in the fast path. We have avoided this thus far
for this reason.

> I don't know if all linux release like ubuntu enable kfence or not.
> I only know it turn on default on android device.

This is irrelevant.

> > While I think the change itself would be useful to have eventually, a
> > better design might be needed. It's unclear to me what the perf impact
>
> Could you share the better design idea ?

Hot-patchable constants, similar to static branches/jump labels. This
had been discussed in the past (can't find the link now), but it's not
trivial to implement unfortunately.

> > is these days (a lot has changed since that comment was written). Could
> > you run some benchmarks to analyze if the fast path is affected by the
> > additional load (please do this for whichever arch you care about, but
> > also arm64 and x86)?
> >
> > If performance is affected, all this could be guarded behind another
> > Kconfig option, but it's not great either.
>
> what kind of option ?
> It already have kconfig option to define the number of objects, here just provide
> a parameter for the same option which user can change.

An option that would enable/disable the command-line changeable number
of objects, i.e one version that avoids the load in the fast path and
one version that enables all the bits that you added here. But I'd
rather avoid this if possible.

As such, please do benchmark and analyze the generated code in the
allocator fast path (you should see a load to the new global you
added). llvm-mca [1] might help you with analysis.

[1] https://llvm.org/docs/CommandGuide/llvm-mca.html

Powered by blists - more mailing lists