Message-ID: <CACT4Y+ZOaVz_EUa-KuMU392Q_TokCpQLv7schd1kXQ_bB_02nA@mail.gmail.com>
Date:   Mon, 20 Jun 2022 14:02:00 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Marco Elver <elver@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Andrey Konovalov <andreyknvl@...il.com>,
        kasan-dev@...glegroups.com
Subject: Re: [RFC PATCH 1/1] lib/stackdepot: replace CONFIG_STACK_HASH_ORDER
 with automatic sizing

On Mon, 20 Jun 2022 at 14:00, Vlastimil Babka <vbabka@...e.cz> wrote:
>
> On 5/27/22 14:02, Dmitry Vyukov wrote:
> > On Fri, 27 May 2022 at 13:37, Vlastimil Babka <vbabka@...e.cz> wrote:
> >>
> >> As Linus explained [1], setting the stackdepot hash table size as a
> >> config option is suboptimal, especially as stackdepot becomes a
> >> dependency of less specialized subsystems than initially (e.g. DRM,
> >> networking, SLUB_DEBUG):
> >>
> >> : (a) it introduces a new compile-time question that isn't sane to ask
> >> : a regular user, but is now exposed to regular users.
> >>
> >> : (b) this by default uses 1MB of memory for a feature that didn't in
> >> : the past, so now if you have small machines you need to make sure you
> >> : make a special kernel config for them.
> >>
> >> Ideally we would employ rhashtable for fully automatic resizing, which
> >> should be feasible for many of the new users, but problematic for the
> >> original users with restricted context that call __stack_depot_save()
> >> with can_alloc == false, i.e. KASAN.
> >>
> >> However we can easily remove the config option and scale the hash table
> >> automatically with system memory. The STACK_HASH_MASK constant becomes
> >> stack_hash_mask variable and is used only in one mask operation, so the
> >> overhead should be negligible to none. For early allocation we can
> >> employ the existing alloc_large_system_hash() function and perform
> >> similar scaling for the late allocation.
> >>
> >> The existing limits of the config option (between 4k and 1M buckets)
> >> are preserved, and scaling factor is set to one bucket per 16kB memory
> >> so on 64bit the max 1M buckets (8MB memory) is achieved with 16GB
> >> system, while a 1GB system will use 512kB.
> >
> > Hi Vlastimil,
> >
> > We use KASAN with VMs with 2GB of memory.
> > If I did the math correctly this will result in 128K entries, while
> > currently we have CONFIG_STACK_HASH_ORDER=20 even for arm32.
> > I am actually not sure how full the table gets, but we can fuzz a
> > large kernel for up to an hour, so we can get lots of stacks (we were
> > the only known users who routinely overflowed default LOCKDEP tables
> > :)).
>
> Aha, good to know the order of 20 has some real use case then :)
>
> > I am not opposed to this in general. And I understand that KASAN Is
> > different from the other users.
> > What do you think re allowing CONFIG_STACK_HASH_ORDER=0/is not set
> > which will mean auto-size, but keeping ability to set exact size as
> > well?
> > Or alternatively auto-size if KASAN is not enabled and use a large
> > table otherwise? But I am not sure if anybody used
> > CONFIG_STACK_HASH_ORDER to reduce the default size with KASAN...
>
> Well if you're unsure and nobody else requested it so far, we could try
> setting it to 20 when KASAN is enabled, and autosize otherwise. If somebody
> comes up with a use-case for the boot-time parameter override (instead of
> CONFIG_), we can add it then?
> >> If needed, the automatic scaling could be complemented with a boot-time
> >> kernel parameter, but it feels pointless to add it without a specific
> >> use case.

Works for me.

Powered by blists - more mailing lists