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 A806DCD379F for ; Fri, 15 Sep 2023 20:32:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 241706B03E2; Fri, 15 Sep 2023 16:32:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F0A06B03E5; Fri, 15 Sep 2023 16:32:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0BA186B03E7; Fri, 15 Sep 2023 16:32:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id EFBCB6B03E2 for ; Fri, 15 Sep 2023 16:32:04 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C90CF1207C2 for ; Fri, 15 Sep 2023 20:32:04 +0000 (UTC) X-FDA: 81239978568.03.BD4A8B4 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by imf11.hostedemail.com (Postfix) with ESMTP id EE0B440015 for ; Fri, 15 Sep 2023 20:32:02 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=LA309tO9; spf=pass (imf11.hostedemail.com: domain of elver@google.com designates 209.85.128.45 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694809923; 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=6aCgnXJdDdEagEQtDfeKQJl/I/7HuJOZio3xGVS/z3M=; b=a1//Bj+3fgiOQSYrtiZq6d5hmUTLDMRVzuksf2PaHTQQJarhD0ffgnu2PrZP/+Y1vggcVM C4Zmyfzj/D5xhzsBcajAY9UmgUesOed+3QS8lhGVWMqpjmJePJuzHm/NimKCd03smbakSl VCZYDdUxfYYA4BEIq9cFrgTQcT0jkic= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694809923; a=rsa-sha256; cv=none; b=8VDIawwDwq6HD5SAoNgUazSa8/PtKV9Razy7jnrLAe1gb2YsGTVd5ugWrJy2wFRBlqJ1Vj BapaHtSOKnW/yF/fnj/P8RQg8OizP6YIV+U00XMWWTdlWyiEX5bRCgF1uz+xSgRh7eBGns AT+DzsV5fW9Nll/Tw/ghZs24OhDz62g= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=LA309tO9; spf=pass (imf11.hostedemail.com: domain of elver@google.com designates 209.85.128.45 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-403012f27e3so29300455e9.3 for ; Fri, 15 Sep 2023 13:32:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1694809921; x=1695414721; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=6aCgnXJdDdEagEQtDfeKQJl/I/7HuJOZio3xGVS/z3M=; b=LA309tO9ezh9lb4Lhb8HwLC7Hs+Br2kkh6+rR+WJs+KwS1O8aA9aNOV/Y5qSY/HdNc tOddQozWMhf1uvUnjjWkCbQUfVeENG9ADf7d5RLQ23Z7F/81h12gThnWt0r57Ww4FEgb 9LMX2gPSSntFKuX8zidzMTZxWe7fgbX3UsuxTm/4/KXM+rOPVcAjz9jCzAU4ed4Hkd3L bwoF2o0Zi3jVtGtbwt3bCXN/YrN8MisSW5RhJOyOHb2wnY4oV4ACXG0etswI96IScumx zlMRehumMgf0az6vvP3g5k2HibhNTxjSj7Ecp8n38uJ6s73rUX3RQBAQjsq9fu8vfFxN k1Iw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694809921; x=1695414721; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=6aCgnXJdDdEagEQtDfeKQJl/I/7HuJOZio3xGVS/z3M=; b=gizxKtuYGIuwxY7BXTmeSy9Y7dtVCJNgzOHXIw7pbl5/S4ksqM7PxRW1EnCKcNdJT8 Xao0FKBxFc+egRDdj/Z2bMTdisEi9cBg0YCl13872xBq5qXIjMwv6lL9O8IdvaOVPDDx bVeTj0sktvHkFECU99+NeicIq/gm4YQY3mYENDTkdz7dgbaBuA4+zOMjGZ4gaAofkJSA zqANLT1crCZ/dWi3012PmkPDM9ixyQgSjk8Dga2qvAljhKj++As7kBbEUzo4W0IcNN5i 1eaeYWt2R5OeOtYPW5NZ+99XHn7jKKKuM7N7Vr6QBdenABloI6mZwByiibK7zImW7ta7 19CA== X-Gm-Message-State: AOJu0YxU5GTwa7m7hzkyxJ0EeihJB1vkEJNOi1K30O/9yrHOCisWnEwJ C0mtzs2BvQ4Zz/saYsliPjuh63RzsTq0id3a4F2FKQ== X-Google-Smtp-Source: AGHT+IHXlZWeboYKCdSUk8xYCnhy/FHNE3Wtss0LF7Ap6Vf14sASPw1xD017cHSoPgyJhdjxZVFRNVWk7Bi69nR1WdU= X-Received: by 2002:a05:600c:3641:b0:401:b652:b6cf with SMTP id y1-20020a05600c364100b00401b652b6cfmr2697790wmq.13.1694809921161; Fri, 15 Sep 2023 13:32:01 -0700 (PDT) MIME-Version: 1.0 References: <2a161c99c47a45f8e9f7a21a732c60f0cd674a66.1694625260.git.andreyknvl@google.com> In-Reply-To: <2a161c99c47a45f8e9f7a21a732c60f0cd674a66.1694625260.git.andreyknvl@google.com> From: Marco Elver Date: Fri, 15 Sep 2023 22:31:21 +0200 Message-ID: Subject: Re: [PATCH v2 14/19] lib/stackdepot, kasan: add flags to __stack_depot_save and rename To: andrey.konovalov@linux.dev Cc: Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vlastimil Babka , kasan-dev@googlegroups.com, Evgenii Stepanov , Oscar Salvador , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrey Konovalov Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: EE0B440015 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: qugq49m1zfbrrrd5rk6sdyqiz66dxtmk X-HE-Tag: 1694809922-561531 X-HE-Meta: U2FsdGVkX1+ky6ZbWkLI45QUZfmuPpqkcKHGSk2PYH3NzYC56sRFRjjUCbDOrd/HHSP2AimdVf4mIPs8WrwcZyEOWjIGKdXppoS5wNWn049IsjEe4Lt/GNdFJsW4KaBwAnowMkVM8xDyQsouDLwt7vmeRabTgLYapImwJGKjcRwDrvjVWWCfVsHEjTiZMXSq7w+pVbtQjkmkYA/S8lrdwVcPAxP5JVElld4svyOyb+882opll8/mRAPMmHntrg2hkE5xlU3Tvlkbp3l9qY8ywWW1vUM6Ji/3WP2L7irt5n0Px4QnnPi0XOCl/uGo6IG0aL/tx1yo+8kLUwNg04yH9m2CZ/q950mCcNTY97Qs+wrcpvv1INSn8RyI0AzOOB2d8UWbrUmfgUwkP1E5746zjtSGO46jJIQ+sfuxI/NQHNoJg178Hi+Re1v03/UTc5IjgbZZTCXVOv1NIbZg2vYlrzG5uDqO2DQo9+9DVUWFP5rS3InE9C3ddQnZFR5oS27F2SCnr+F+XghM5nl0jKG90kxomc8LaTSVg0FOnJ3A052uaOUC32K89MNLvqUTwaEqs9HN70qmdKhNvUPrdKyEkh7dVKnI5Tp6FKYuLvE0YphqjVa8xcNCBuVbPVmA6ROKUAbAIlrAMmNhgnZyQiLfUulJMmabiyEiEdoqgIiR7U5S/5ECZDp/QO+MTDI2GiNCIaadtHmVuTCV8ZAnhoehmaDd2pEt+rFezTH+1NLBAbm1A/RgK16vMm/o39OPPCoKrrEqaaiujJfIuZ77/oxXxzgHW3UPWubAHEx2RQFglaTHi3Nhq7vXOaHcsh8LAem+apng/ODcQEH3nG0EkPE7BquUWfnHxxYc02EV4nFBvMayvwghcTbQW1yL8/q7IB79J1qs1uhehk9eLQuBJ0E15wIS9s/3w1bQuj9ShhCVA9hPLdgms0ii8pwcA8qx+afCuPvuArn6jgdIB71rumL nH4fv47U NpwpjYppVHllVFLvjOIn40zc+l/W4duN1kqARjS44tUi0MXi6eXlBioC+yeozvur6XLygWPC9Z/G4Wp/3v5ypwCZn43UgkRN5hrITdYWzrmuQRxpZOPx424rfpP3FM5uZ5Ku6YgVOExCUADhqHVGalDUqhCDu3cgB4YKkwNT5r3b4CNyRQxefZvNZZadHkJVQDaIYVXfZhyUPIHh2rWCa4oS+gEa6VWGC+dpm/PYxn0JVHTp99QVwarnr9kDfTd0vKcCtxZRSh6nir/P9NHJhK/76+tH+FOmRdze25Si+2b3x2Vg= 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 Wed, 13 Sept 2023 at 19:17, wrote: > > From: Andrey Konovalov > > Change the bool can_alloc argument of __stack_depot_save to a > u32 argument that accepts a set of flags. > > The following patch will add another flag to stack_depot_save_flags > besides the existing STACK_DEPOT_FLAG_CAN_ALLOC. > > Also rename the function to stack_depot_save_flags, as __stack_depot_save > is a cryptic name, > > Signed-off-by: Andrey Konovalov > > --- > > Changes v1->v2: > - This is a new patch. > --- > include/linux/stackdepot.h | 36 +++++++++++++++++++++++++----------- > lib/stackdepot.c | 16 +++++++++++----- > mm/kasan/common.c | 7 ++++--- > mm/kasan/generic.c | 9 +++++---- > mm/kasan/kasan.h | 2 +- > mm/kasan/tags.c | 3 ++- > 6 files changed, 48 insertions(+), 25 deletions(-) > > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h > index e58306783d8e..0b262e14144e 100644 > --- a/include/linux/stackdepot.h > +++ b/include/linux/stackdepot.h > @@ -32,6 +32,17 @@ typedef u32 depot_stack_handle_t; > */ > #define STACK_DEPOT_EXTRA_BITS 5 > > +typedef u32 depot_flags_t; > + > +/* > + * Flags that can be passed to stack_depot_save_flags(); see the comment next > + * to its declaration for more details. > + */ > +#define STACK_DEPOT_FLAG_CAN_ALLOC ((depot_flags_t)0x0001) > + > +#define STACK_DEPOT_FLAGS_NUM 1 > +#define STACK_DEPOT_FLAGS_MASK ((depot_flags_t)((1 << STACK_DEPOT_FLAGS_NUM) - 1)) > + > /* > * Using stack depot requires its initialization, which can be done in 3 ways: > * > @@ -69,31 +80,34 @@ static inline int stack_depot_early_init(void) { return 0; } > #endif > > /** > - * __stack_depot_save - Save a stack trace to stack depot > + * stack_depot_save_flags - Save a stack trace to stack depot > * > * @entries: Pointer to the stack trace > * @nr_entries: Number of frames in the stack > * @alloc_flags: Allocation GFP flags > - * @can_alloc: Allocate stack pools (increased chance of failure if false) > + * @depot_flags: Stack depot flags > + * > + * Saves a stack trace from @entries array of size @nr_entries. > * > - * Saves a stack trace from @entries array of size @nr_entries. If @can_alloc is > - * %true, stack depot can replenish the stack pools in case no space is left > - * (allocates using GFP flags of @alloc_flags). If @can_alloc is %false, avoids > - * any allocations and fails if no space is left to store the stack trace. > + * If STACK_DEPOT_FLAG_CAN_ALLOC is set in @depot_flags, stack depot can > + * replenish the stack pools in case no space is left (allocates using GFP > + * flags of @alloc_flags). Otherwise, stack depot avoids any allocations and > + * fails if no space is left to store the stack trace. > * > * If the provided stack trace comes from the interrupt context, only the part > * up to the interrupt entry is saved. > * > - * Context: Any context, but setting @can_alloc to %false is required if > + * Context: Any context, but setting STACK_DEPOT_FLAG_CAN_ALLOC is required if > * alloc_pages() cannot be used from the current context. Currently > * this is the case for contexts where neither %GFP_ATOMIC nor > * %GFP_NOWAIT can be used (NMI, raw_spin_lock). > * > * Return: Handle of the stack struct stored in depot, 0 on failure > */ > -depot_stack_handle_t __stack_depot_save(unsigned long *entries, > - unsigned int nr_entries, > - gfp_t gfp_flags, bool can_alloc); > +depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > + unsigned int nr_entries, > + gfp_t gfp_flags, > + depot_flags_t depot_flags); > > /** > * stack_depot_save - Save a stack trace to stack depot > @@ -103,7 +117,7 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > * @alloc_flags: Allocation GFP flags > * > * Context: Contexts where allocations via alloc_pages() are allowed. > - * See __stack_depot_save() for more details. > + * See stack_depot_save_flags() for more details. > * > * Return: Handle of the stack trace stored in depot, 0 on failure > */ > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 1b08897ebd2b..e5121225f124 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -438,19 +438,24 @@ static inline struct stack_record *find_stack(struct list_head *bucket, > return NULL; > } > > -depot_stack_handle_t __stack_depot_save(unsigned long *entries, > - unsigned int nr_entries, > - gfp_t alloc_flags, bool can_alloc) > +depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > + unsigned int nr_entries, > + gfp_t alloc_flags, > + depot_flags_t depot_flags) > { > struct list_head *bucket; > struct stack_record *found = NULL; > depot_stack_handle_t handle = 0; > struct page *page = NULL; > void *prealloc = NULL; > + bool can_alloc = depot_flags & STACK_DEPOT_FLAG_CAN_ALLOC; > bool need_alloc = false; > unsigned long flags; > u32 hash; > > + if (depot_flags & ~STACK_DEPOT_FLAGS_MASK) > + return 0; > + Shouldn't this be a WARN due to invalid flags? > /* > * If this stack trace is from an interrupt, including anything before > * interrupt entry usually leads to unbounded stack depot growth. > @@ -529,13 +534,14 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > handle = found->handle.handle; > return handle; > } > -EXPORT_SYMBOL_GPL(__stack_depot_save); > +EXPORT_SYMBOL_GPL(stack_depot_save_flags); > > depot_stack_handle_t stack_depot_save(unsigned long *entries, > unsigned int nr_entries, > gfp_t alloc_flags) > { > - return __stack_depot_save(entries, nr_entries, alloc_flags, true); > + return stack_depot_save_flags(entries, nr_entries, alloc_flags, > + STACK_DEPOT_FLAG_CAN_ALLOC); > } > EXPORT_SYMBOL_GPL(stack_depot_save); > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > index 256930da578a..825a0240ec02 100644 > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -37,19 +38,19 @@ struct slab *kasan_addr_to_slab(const void *addr) > return NULL; > } > > -depot_stack_handle_t kasan_save_stack(gfp_t flags, bool can_alloc) > +depot_stack_handle_t kasan_save_stack(gfp_t flags, depot_flags_t depot_flags) > { > unsigned long entries[KASAN_STACK_DEPTH]; > unsigned int nr_entries; > > nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0); > - return __stack_depot_save(entries, nr_entries, flags, can_alloc); > + return stack_depot_save_flags(entries, nr_entries, flags, depot_flags); > } > > void kasan_set_track(struct kasan_track *track, gfp_t flags) > { > track->pid = current->pid; > - track->stack = kasan_save_stack(flags, true); > + track->stack = kasan_save_stack(flags, STACK_DEPOT_FLAG_CAN_ALLOC); > } > > #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index 4d837ab83f08..5d168c9afb32 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -472,7 +473,7 @@ size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object) > sizeof(struct kasan_free_meta) : 0); > } > > -static void __kasan_record_aux_stack(void *addr, bool can_alloc) > +static void __kasan_record_aux_stack(void *addr, depot_flags_t depot_flags) > { > struct slab *slab = kasan_addr_to_slab(addr); > struct kmem_cache *cache; > @@ -489,17 +490,17 @@ static void __kasan_record_aux_stack(void *addr, bool can_alloc) > return; > > alloc_meta->aux_stack[1] = alloc_meta->aux_stack[0]; > - alloc_meta->aux_stack[0] = kasan_save_stack(0, can_alloc); > + alloc_meta->aux_stack[0] = kasan_save_stack(0, depot_flags); > } > > void kasan_record_aux_stack(void *addr) > { > - return __kasan_record_aux_stack(addr, true); > + return __kasan_record_aux_stack(addr, STACK_DEPOT_FLAG_CAN_ALLOC); > } > > void kasan_record_aux_stack_noalloc(void *addr) > { > - return __kasan_record_aux_stack(addr, false); > + return __kasan_record_aux_stack(addr, 0); > } > > void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags) > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index f70e3d7a602e..de3206e11888 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -370,7 +370,7 @@ static inline void kasan_init_cache_meta(struct kmem_cache *cache, unsigned int > static inline void kasan_init_object_meta(struct kmem_cache *cache, const void *object) { } > #endif > > -depot_stack_handle_t kasan_save_stack(gfp_t flags, bool can_alloc); > +depot_stack_handle_t kasan_save_stack(gfp_t flags, depot_flags_t depot_flags); > void kasan_set_track(struct kasan_track *track, gfp_t flags); > void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags); > void kasan_save_free_info(struct kmem_cache *cache, void *object); > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c > index 7dcfe341d48e..4fd32121b0fd 100644 > --- a/mm/kasan/tags.c > +++ b/mm/kasan/tags.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -101,7 +102,7 @@ static void save_stack_info(struct kmem_cache *cache, void *object, > struct kasan_stack_ring_entry *entry; > void *old_ptr; > > - stack = kasan_save_stack(gfp_flags, true); > + stack = kasan_save_stack(gfp_flags, STACK_DEPOT_FLAG_CAN_ALLOC); > > /* > * Prevent save_stack_info() from modifying stack ring > -- > 2.25.1 >