Re: [RFC PATCH 0/2] Alloc kfence_pool after system startup

[Tianchen Ding]

On 2022/3/3 17:30, Marco Elver wrote:

Thanks for your replies.
I do see setting a large sample_interval means almost disabling KFENCE.
In fact, my point is to provide a more “flexible” way. Since some Ops 
may be glad to use something like on/off switch than 10000ms interval. :-)

> On Thu, 3 Mar 2022 at 10:05, Alexander Potapenko <glider@xxxxxxxxxx> wrote:
>
> I share Alex's concerns.
>
> > On Thu, Mar 3, 2022 at 4:15 AM Tianchen Ding <dtcccc@xxxxxxxxxxxxxxxxx> wrote:
> > > KFENCE aims at production environments, but it does not allow enabling
> > > after system startup because kfence_pool only alloc pages from memblock.
> > > Consider the following production scene:
> > > At first, for performance considerations, production machines do not
> > > enable KFENCE.
> >
> > What are the performance considerations you have in mind? Are you running KFENCE with a very aggressive sampling rate?
>
> Indeed, what is wrong with simply starting up KFENCE with a sample
> interval of 10000? However, I very much doubt that you'll notice any
> performance issues above 500ms.
>
> Do let us know what performance issues you have seen. It may be
> related to an earlier version of KFENCE but has since been fixed (see
> log).
>
> > > However, after running for a while, the kernel is suspected to have
> > > memory errors. (e.g., a sibling machine crashed.)
> >
> > I have doubts regarding this setup. It might be faster (although one can tune KFENCE to have nearly zero performance impact), but is harder to maintain.
> > It will also catch fewer errors than if you just had KFENCE on from the very beginning:
> >   - sibling machines may behave differently, and a certain bug may only occur once - in that case the secondary instances won't notice it, even with KFENCE;
> >   - KFENCE also catches non-lethal corruptions (e.g. OOB reads), which may stay under radar for a very long time.
> >
> > > So other production machines need to enable KFENCE, but it's hard for
> > > them to reboot.
> > >
> > > The 1st patch allows re-enabling KFENCE if the pool is already
> > > allocated from memblock.
>
> Patch 1/2 might be ok by itself, but I still don't see the point
> because you should just leave KFENCE enabled. There should be no
> reason to have to turn it off. If anything, you can increase the
> sample interval to something very large if needed.