From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14B6EC433EF for ; Tue, 21 Jun 2022 07:37:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7CBDE8E0003; Tue, 21 Jun 2022 03:37:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 72C6A6B007D; Tue, 21 Jun 2022 03:37:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5CD358E0003; Tue, 21 Jun 2022 03:37:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 489CE6B007B for ; Tue, 21 Jun 2022 03:37:31 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 1B7B421147 for ; Tue, 21 Jun 2022 07:37:31 +0000 (UTC) X-FDA: 79601437902.05.A1262A6 Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf03.hostedemail.com (Postfix) with ESMTP id A5A11200AB for ; Tue, 21 Jun 2022 07:37:30 +0000 (UTC) Received: by mail-lj1-f180.google.com with SMTP id o23so7280921ljg.13 for ; Tue, 21 Jun 2022 00:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CyheJD4bAlrZUyrOZt+A8Zzf01u3nd0p7PNq0BkSQPg=; b=DsougCXdnJ9//fXVMi81UM/OA1cGshIeJkqJrhKSzVNw1NIKiJ4Q9vlHofpRGGkaRq A6hIVLyao9AHHANCZIo87cb9zhjJJ1xPqnICDZKL+GYF4+iQIHxQAmvnxLSS8LBFKVDM x/RvFpNgsYT2pdsmihw8nd8IiBYurW+KqwmEoAmu6+SvvMaC0XzvlSSLJlDRZCjENleG Y0T+BozcVt2J0evJsQtmYvlLaWyes4TRQFEKPoS6YmdSUpxgddH/DeCgigifm0Z1gtgU AjNd3OxOLr1Va4xSSx7iWXVqnDVaImlhk1D6CO1zpz25bUPxi3LR/ohmbNE04RpUG59K DNgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CyheJD4bAlrZUyrOZt+A8Zzf01u3nd0p7PNq0BkSQPg=; b=AXOpirkvVyZkMoV6bA/IGkR76WjAOnsdzf0Ja7iEf9H0xrLaerOyGHbAGqLQ/vOeHT tjVX9aFwYQvu+QuPVULzshMzGLv5c1bz373j9SWrXsr1E0fc+XzMBc8hN0g9K898ezjd KrJD29lNhZkzql9DYnM7HtYnQ9tLF76bmQ2Xjl38DORhHU/Di5VLg5PDai1qIEnmW7Ou Y2X49KatTGPfFyXrUnfb3UrVKQWH/FXAKyafeEW8coHRNyJYUqlMM2BNa8Q3wJGFaoy1 +A3urDy6d0bOrBGjODL2UeOtlrWA01sQ4WjQOWg+PjgztqHntG3whwhCuoLwXK6Ae/am OviQ== X-Gm-Message-State: AJIora8reUbacCvCtqVcVcvlvuiUbHvjpVancDIn+gg6OTsgeK2iwIbw oRB3XrV2CFuQvSLBT+Yai76lyCpBtnfNMo9azWqR5g== X-Google-Smtp-Source: AGRyM1sYg9IbMSQyU3FZlgEitV/tUOop/c49ZQm9/duohXtdXd4fEoVURd7nS4L3G5Y8nHZagAc/TnboivK4BlVYHGE= X-Received: by 2002:a05:651c:1988:b0:255:b2ef:6a5b with SMTP id bx8-20020a05651c198800b00255b2ef6a5bmr13840428ljb.465.1655797048804; Tue, 21 Jun 2022 00:37:28 -0700 (PDT) MIME-Version: 1.0 References: <20220527113706.24870-1-vbabka@suse.cz> <20220620150249.16814-1-vbabka@suse.cz> In-Reply-To: <20220620150249.16814-1-vbabka@suse.cz> From: Dmitry Vyukov Date: Tue, 21 Jun 2022 09:37:17 +0200 Message-ID: Subject: Re: [PATCH] lib/stackdepot: replace CONFIG_STACK_HASH_ORDER with automatic sizing To: Vlastimil Babka Cc: Marco Elver , Alexander Potapenko , Andrew Morton , Linus Torvalds , linux-mm@kvack.org, linux-kernel@vger.kernel.org, patches@lists.linux.dev, Andrey Konovalov , kasan-dev@googlegroups.com Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=DsougCXd; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of dvyukov@google.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=dvyukov@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655797050; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=CyheJD4bAlrZUyrOZt+A8Zzf01u3nd0p7PNq0BkSQPg=; b=6bAJhWSrIViiOtZtrzt/PwDxoLG3IWRhW3kg4kS0I3UXJ9szwBZ+j5rnK5H3v0ZbFmQj4E Ow5wiNVYzFRMQQ6Zs5X/lwMuJnrGNHY7PbXHkYMfoXcV9+HyiRjUk9Z2SzUUQengMah+5e w0FxJXzAK0buv3O9NJcFG8dV7Gq/TdU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655797050; a=rsa-sha256; cv=none; b=G9OQYDns17pF+p/ku8Xu8LwIkAzUuY8PEIZ3wxVoqibSYKYyckYRoiiHRTfd7JfXqWG2+n 8Vx5meI9ur99N6rlO7u7E30nzZS7BQrubKnJY5pdn4XazYgJGNgO6UA1J1XQhwBI0EbNXU RvrAs/F8/4GQU0nkYXARcKQ2XTmND/Q= X-Rspamd-Queue-Id: A5A11200AB Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=DsougCXd; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of dvyukov@google.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=dvyukov@google.com X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: mbypmzzee5j67ge4ecinzntcsyfnj1kh X-HE-Tag: 1655797050-651351 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, 20 Jun 2022 at 17:03, Vlastimil Babka 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 "expert" 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. > > Because KASAN is reported to need the maximum number of buckets even > with smaller amounts of memory [2], set it as such when kasan_enabled(). > > 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. > > [1] https://lore.kernel.org/all/CAHk-=wjC5nS+fnf6EzRD9yQRJApAhxx7gRB87ZV+pAWo9oVrTg@mail.gmail.com/ > [2] https://lore.kernel.org/all/CACT4Y+Y4GZfXOru2z5tFPzFdaSUd+GFc6KVL=bsa0+1m197cQQ@mail.gmail.com/ > > Reported-by: Linus Torvalds > Signed-off-by: Vlastimil Babka Acked-by: Dmitry Vyukov > --- > lib/Kconfig | 9 -------- > lib/stackdepot.c | 59 ++++++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 49 insertions(+), 19 deletions(-) > > diff --git a/lib/Kconfig b/lib/Kconfig > index eaaad4d85bf2..986ea474836c 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -685,15 +685,6 @@ config STACKDEPOT_ALWAYS_INIT > bool > select STACKDEPOT > > -config STACK_HASH_ORDER > - int "stack depot hash size (12 => 4KB, 20 => 1024KB)" > - range 12 20 > - default 20 > - depends on STACKDEPOT > - help > - Select the hash size as a power of 2 for the stackdepot hash table. > - Choose a lower value to reduce the memory impact. > - > config REF_TRACKER > bool > depends on STACKTRACE_SUPPORT > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 5ca0d086ef4a..e73fda23388d 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -32,6 +32,7 @@ > #include > #include > #include > +#include > > #define DEPOT_STACK_BITS (sizeof(depot_stack_handle_t) * 8) > > @@ -145,10 +146,16 @@ depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) > return stack; > } > > -#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER) > -#define STACK_HASH_MASK (STACK_HASH_SIZE - 1) > +/* one hash table bucket entry per 16kB of memory */ > +#define STACK_HASH_SCALE 14 > +/* limited between 4k and 1M buckets */ > +#define STACK_HASH_ORDER_MIN 12 > +#define STACK_HASH_ORDER_MAX 20 > #define STACK_HASH_SEED 0x9747b28c > > +static unsigned int stack_hash_order; > +static unsigned int stack_hash_mask; > + > static bool stack_depot_disable; > static struct stack_record **stack_table; > > @@ -175,7 +182,7 @@ void __init stack_depot_want_early_init(void) > > int __init stack_depot_early_init(void) > { > - size_t size; > + unsigned long entries = 0; > > /* This is supposed to be called only once, from mm_init() */ > if (WARN_ON(__stack_depot_early_init_passed)) > @@ -183,13 +190,23 @@ int __init stack_depot_early_init(void) > > __stack_depot_early_init_passed = true; > > + if (kasan_enabled() && !stack_hash_order) > + stack_hash_order = STACK_HASH_ORDER_MAX; > + > if (!__stack_depot_want_early_init || stack_depot_disable) > return 0; > > - size = (STACK_HASH_SIZE * sizeof(struct stack_record *)); > - pr_info("Stack Depot early init allocating hash table with memblock_alloc, %zu bytes\n", > - size); > - stack_table = memblock_alloc(size, SMP_CACHE_BYTES); > + if (stack_hash_order) > + entries = 1UL << stack_hash_order; > + stack_table = alloc_large_system_hash("stackdepot", > + sizeof(struct stack_record *), > + entries, > + STACK_HASH_SCALE, > + HASH_EARLY | HASH_ZERO, > + NULL, > + &stack_hash_mask, > + 1UL << STACK_HASH_ORDER_MIN, > + 1UL << STACK_HASH_ORDER_MAX); > > if (!stack_table) { > pr_err("Stack Depot hash table allocation failed, disabling\n"); > @@ -207,13 +224,35 @@ int stack_depot_init(void) > > mutex_lock(&stack_depot_init_mutex); > if (!stack_depot_disable && !stack_table) { > - pr_info("Stack Depot allocating hash table with kvcalloc\n"); > - stack_table = kvcalloc(STACK_HASH_SIZE, sizeof(struct stack_record *), GFP_KERNEL); > + unsigned long entries; > + int scale = STACK_HASH_SCALE; > + > + if (stack_hash_order) { > + entries = 1UL << stack_hash_order; > + } else { > + entries = nr_free_buffer_pages(); > + entries = roundup_pow_of_two(entries); > + > + if (scale > PAGE_SHIFT) > + entries >>= (scale - PAGE_SHIFT); > + else > + entries <<= (PAGE_SHIFT - scale); > + } > + > + if (entries < 1UL << STACK_HASH_ORDER_MIN) > + entries = 1UL << STACK_HASH_ORDER_MIN; > + if (entries > 1UL << STACK_HASH_ORDER_MAX) > + entries = 1UL << STACK_HASH_ORDER_MAX; > + > + pr_info("Stack Depot allocating hash table of %lu entries with kvcalloc\n", > + entries); > + stack_table = kvcalloc(entries, sizeof(struct stack_record *), GFP_KERNEL); > if (!stack_table) { > pr_err("Stack Depot hash table allocation failed, disabling\n"); > stack_depot_disable = true; > ret = -ENOMEM; > } > + stack_hash_mask = entries - 1; > } > mutex_unlock(&stack_depot_init_mutex); > return ret; > @@ -386,7 +425,7 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > goto fast_exit; > > hash = hash_stack(entries, nr_entries); > - bucket = &stack_table[hash & STACK_HASH_MASK]; > + bucket = &stack_table[hash & stack_hash_mask]; > > /* > * Fast path: look the stack trace up without locking. > -- > 2.36.1 >