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 X-Spam-Level: X-Spam-Status: No, score=-15.1 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1BEF5C433B4 for ; Thu, 13 May 2021 10:31:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 775A06143E for ; Thu, 13 May 2021 10:31:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 775A06143E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C13FC6B0036; Thu, 13 May 2021 06:31:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BC32C6B006E; Thu, 13 May 2021 06:31:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A3D956B0070; Thu, 13 May 2021 06:31:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0084.hostedemail.com [216.40.44.84]) by kanga.kvack.org (Postfix) with ESMTP id 705746B0036 for ; Thu, 13 May 2021 06:31:47 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 071AB1803F495 for ; Thu, 13 May 2021 10:31:47 +0000 (UTC) X-FDA: 78135841854.09.0852154 Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by imf29.hostedemail.com (Postfix) with ESMTP id C1E8D780 for ; Thu, 13 May 2021 10:31:39 +0000 (UTC) Received: by mail-wr1-f53.google.com with SMTP id x5so26416625wrv.13 for ; Thu, 13 May 2021 03:31:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=I9TVKnEoxbGOXSQCVpBWnu5KN6jl7TtGegYCc95oDNQ=; b=E+2irKwtpsfPWTsIjd+6TJi7qEFHdmPqtExPYrmLdRs5MfegtRmP/2vOt4TCTFEPvm sjWbZsO0RkbOk9TBGl7Er/3A4a+GIXAmSu7JOpc6AM4kZ/mOuZ03Rr/puuxg2idaNBjg n70IW34QLegjjClx45zq7/ptvQSHshhRE/JbjcOdGRe81dlpzYw6/MVkkigt0R6L3VaP X5Ght54Ddxtzzq+odoHUOr0cHnBEP45PlZBKshksGVXMvCGwmqxWM3ChbBw3WDTEv78Z on8yeGkCU0KPoSohT5hCcN2gYBETAWLe4mmhNjO42VlDMjE29grku0IvZGwiAqnfM5em J/Ig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=I9TVKnEoxbGOXSQCVpBWnu5KN6jl7TtGegYCc95oDNQ=; b=O6Ob1WznhEgmx6/cThqTkcyppbLGxTpJK1qsIwFiF3ZNlZ+b620+eoVmcbvUjnLQdT ZdyvZN3JsRDaBBeowY9JhLgt6w8l5sFH1UKlbIaZJ50EkgGI0fSJYsOrwVYFjE9AtJNB cUG1FmZ/uxx6DQ8x6Maq0e8EaZpkdV3EKMv55z5u+FF28fm1vwfG3dXHVqJ1EiFWrnbM D1rC97EBKtqrs7mhjHdkotxRWsoVm7NYjL9pIRv1h9A3m/746/vweLm4Dd2g0UwMLZZo g4gGGZjALMRx7s0XE7i1wya787Yha+eRv+/9YVoxflEbX4hGncLjADBFLJMFEk3MU6lq ckiA== X-Gm-Message-State: AOAM532SUPCtRyP4x/zbrphh2wrPf/zMMpTP2m5GoRkVQEPDMF0mlFQO mexH+cL5GtvQHjskiXtEUTGzoQ== X-Google-Smtp-Source: ABdhPJyW1ehp4cfqSjErQlvA+WGvuA1Dx8/t7dGKIZ0TQAM8zv4kkHtcCHFQm4hvzIXVrUCII0dt4g== X-Received: by 2002:a5d:498c:: with SMTP id r12mr51128256wrq.31.1620901905146; Thu, 13 May 2021 03:31:45 -0700 (PDT) Received: from elver.google.com ([2a00:79e0:15:13:c5e0:2ff3:baa4:b294]) by smtp.gmail.com with ESMTPSA id b8sm2529623wrx.15.2021.05.13.03.31.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 May 2021 03:31:44 -0700 (PDT) Date: Thu, 13 May 2021 12:31:38 +0200 From: Marco Elver To: Vlastimil Babka Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>, Andrew Morton , iamjoonsoo.kim@lge.com, rientjes@google.com, penberg@kernel.org, cl@linux.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] mm, slub: change run-time assertion in kmalloc_index() to compile-time Message-ID: References: <20210511173448.GA54466@hyeyoo> <20210512195227.245000695c9014242e9a00e5@linux-foundation.org> <20210513031220.GA133011@hyeyoo> <20210512204024.401ff3de38649d7d0f5a45e8@linux-foundation.org> <20210513062809.GA319973@hyeyoo> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.5 (2021-01-21) X-Rspamd-Queue-Id: C1E8D780 Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=E+2irKwt; spf=pass (imf29.hostedemail.com: domain of elver@google.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam04 X-Stat-Signature: jsgomozc1ddooh8dtbx46kpjkzdrqxnq Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf29; identity=mailfrom; envelope-from=""; helo=mail-wr1-f53.google.com; client-ip=209.85.221.53 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1620901899-427690 Content-Transfer-Encoding: quoted-printable 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 Thu, May 13, 2021 at 10:51AM +0200, Vlastimil Babka wrote: > On 5/13/21 8:28 AM, Hyeonggon Yoo wrote: > > On Wed, May 12, 2021 at 08:40:24PM -0700, Andrew Morton wrote: > >> On Thu, 13 May 2021 12:12:20 +0900 Hyeonggon Yoo <42.hyeyoo@gmail.co= m> wrote: > >> > On Wed, May 12, 2021 at 07:52:27PM -0700, Andrew Morton wrote: > >> > > This explodes in mysterious ways. The patch as I have it is app= ended, > >> > > for reference. > >> > >=20 > >> > > gcc-10.3.0 allmodconfig. > >> > >=20 > >> > > This patch suppresses the error: > >>=20 > >> Ah, yes, of course, your patch changes kmalloc_index() to require th= at > >> it always is called with a constant `size'. kfence_test doesn't do > >> that. > >>=20 > >> kfence is being a bit naughty here - the other kmalloc_index() calle= rs > >> only comple up the call after verifying that `size' is a compile-tim= e > >> constant. >=20 > Agreed. It's just a test, and performance doesn't matter for it. The thing is this function lives in , isn't prefixed with __ or anything like that, so it really does look like a public function. > >> Would something like this work? >=20 > I'd prefer if we kept kmalloc_index() for constant sizes only. The brok= en build > then warns anyone using it the wrong way that they shouldn't. Agreed. Andrew's size_is_constant would do that. Also see my suggestion below to keep the same interface. > Besides, it really > shouldn't be used outside of slab. It's an allocator test. If we want to facilitate testing, it must be allowed to verify or set up test cases that test boundary conditions based on internal state. In the case of kfence_test it wants:=A0 the cache's alignment to create accesses that fall on alignment boundaries; and to verify obj_to_index() and objs_per_slab_page() are set up correctly. I think the requirements are: 1. Make the interface hard to abuse. Adding the BUILD_BUG_ON does that. 2. Facilitate testing. > But if kfence test really needs this, we could perhaps extract the inde= x > determining part out of kmalloc_slab(). That would duplicate kmalloc_index()? I don't see the need, let's keep things simple. > Hmm or I guess the kfence tests could just use kmalloc_slab() directly? kmalloc_slab() is internal to slab and should not be exported. It'd require exporting because the tests can be built as modules. kmalloc_index() works perfectly fine, and the test really doesn't care about performance of kmalloc_index(). :-) See my suggestion below that builds on Andrew's size_is_constant but would retain the old interface and support testing. Thanks, -- Marco ------ >8 ------ From: Marco Elver Subject: [PATCH] kfence: test: fix for "mm, slub: change run-time asserti= on in kmalloc_index() to compile-time" Signed-off-by: Marco Elver --- include/linux/slab.h | 9 +++++++-- mm/kfence/kfence_test.c | 5 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/include/linux/slab.h b/include/linux/slab.h index 27d142564557..7a10bdc4b7a9 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -350,7 +350,8 @@ static __always_inline enum kmalloc_cache_type kmallo= c_type(gfp_t flags) * Note: there's no need to optimize kmalloc_index because it's evaluate= d * in compile-time. */ -static __always_inline unsigned int kmalloc_index(size_t size) +static __always_inline unsigned int __kmalloc_index(size_t size, + bool size_is_constant) { if (!size) return 0; @@ -386,11 +387,15 @@ static __always_inline unsigned int kmalloc_index(s= ize_t size) if (size <=3D 16 * 1024 * 1024) return 24; if (size <=3D 32 * 1024 * 1024) return 25; =20 - BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()"); + if (size_is_constant) + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()"); + else + BUG(); =20 /* Will never be reached. Needed because the compiler may complain */ return -1; } +#define kmalloc_index(s) __kmalloc_index(s, true) #endif /* !CONFIG_SLOB */ =20 void *__kmalloc(size_t size, gfp_t flags) __assume_kmalloc_alignment __m= alloc; diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c index 4acf4251ee04..7f24b9bcb2ec 100644 --- a/mm/kfence/kfence_test.c +++ b/mm/kfence/kfence_test.c @@ -197,7 +197,7 @@ static void test_cache_destroy(void) =20 static inline size_t kmalloc_cache_alignment(size_t size) { - return kmalloc_caches[kmalloc_type(GFP_KERNEL)][kmalloc_index(size)]->a= lign; + return kmalloc_caches[kmalloc_type(GFP_KERNEL)][__kmalloc_index(size, f= alse)]->align; } =20 /* Must always inline to match stack trace against caller. */ @@ -267,7 +267,8 @@ static void *test_alloc(struct kunit *test, size_t si= ze, gfp_t gfp, enum allocat =20 if (is_kfence_address(alloc)) { struct page *page =3D virt_to_head_page(alloc); - struct kmem_cache *s =3D test_cache ?: kmalloc_caches[kmalloc_type(GF= P_KERNEL)][kmalloc_index(size)]; + struct kmem_cache *s =3D test_cache ?: + kmalloc_caches[kmalloc_type(GFP_KERNEL)][__kmalloc_index(size, fals= e)]; =20 /* * Verify that various helpers return the right values --=20 2.31.1.607.g51e8a6a459-goog