On Mon, 20 Jun 2022 at 14:00, Vlastimil Babka <vbabka@xxxxxxx> wrote: > > On 5/27/22 14:02, Dmitry Vyukov wrote: > > On Fri, 27 May 2022 at 13:37, Vlastimil Babka <vbabka@xxxxxxx> 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.