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=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL 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 B5D43C2D0E4 for ; Thu, 12 Nov 2020 23:00:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5FD6220B80 for ; Thu, 12 Nov 2020 23:00:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ZA7ZfI8M" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726231AbgKLXAc (ORCPT ); Thu, 12 Nov 2020 18:00:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54016 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725929AbgKLXAc (ORCPT ); Thu, 12 Nov 2020 18:00:32 -0500 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EEE03C0613D1 for ; Thu, 12 Nov 2020 15:00:31 -0800 (PST) Received: by mail-pf1-x441.google.com with SMTP id q10so5960633pfn.0 for ; Thu, 12 Nov 2020 15:00:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=ZA7ZfI8MWSiXS7AZkGgFD1bkrkW1aNC29iy8wrCtuDTNNQ8d4nb6NqqyRWd0LbZJWx PnT0TxZ5Zn4VvWTa/gvoPhVsdEU5qdFGpnGHLSxPwpioP9gBkJdKa5oglMlNfgKkKIkZ MdU4hXgfTfalu5OsqcgO9UuZsFxbWvtfEb8V19U756z01OtQphAnbJ4O407mpKUopo3k uMKOFpjrziWNQQhBG9v4nl5jA1WDrFt+JS+YcbbGSfSFHrrAtdGZUSPNYgVitRv64Gj3 M1YmfLqkYd26omnlUO+84qjxfnzZXpXBbMFpOPAbAY+HnpG5AtH1O2MtzZzZH+SBpAAk byWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=toUTLcgw+ZNrYuixDFhIzkV/Q5ZOMzvjo6qoJ/TYYcVILOLEtDdC0YMEPDzR43UKoz hhQwLzh5e7q7Bt0XiZ/NHin+RIyIZGYGUGiiFbGVbrdbYGz7ryerTuTWSnLvj7xg91JH Bq2RUg8gY80YpZndUFLDxXn+IVOEsi3TcGH1hfeMk/ComFo5NS0EaLt0bcZK/kvYgyK/ ClkX4NY597QnUPxeVTzSQOyECzTo5KGlqWODBR+h3AZ+LGpLcEvDCJKnwjgoNAGmgus9 pkEMA4SpB9PBKnNkhBbT1YGX6lM0M6I9Q6mSHTdlB1Y4d24Btd52AE9NShXgi0LfNEiP LJ+A== X-Gm-Message-State: AOAM530isPvemqMmlnO7UYoGlU5P41gAkrBg+Ofkq2mzMuFr2iF/kXM/ Qoq6KjQ71ifVMck/E/RY/X/PPQPZJkbPMOpDFeXYhQ== X-Google-Smtp-Source: ABdhPJzhW5OGR3SOnBOJ/bdTcqYxkR+Ie1nyTBybuHelmvZvjTkLyOkpby7U4aWuzcyUFw24aqfQQRmXqCEi9GgaTBA= X-Received: by 2002:a17:90b:3111:: with SMTP id gc17mr30483pjb.41.1605222031183; Thu, 12 Nov 2020 15:00:31 -0800 (PST) MIME-Version: 1.0 References: <936c0c198145b663e031527c49a6895bd21ac3a0.1605046662.git.andreyknvl@google.com> <20201111151336.GA517454@elver.google.com> In-Reply-To: <20201111151336.GA517454@elver.google.com> From: Andrey Konovalov Date: Fri, 13 Nov 2020 00:00:20 +0100 Message-ID: Subject: Re: [PATCH v2 19/20] kasan, mm: allow cache merging with no metadata To: Marco Elver Cc: Dmitry Vyukov , Alexander Potapenko , Catalin Marinas , Will Deacon , Vincenzo Frascino , Evgenii Stepanov , Andrey Ryabinin , Branislav Rankov , Kevin Brodsky , Andrew Morton , kasan-dev , Linux ARM , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 11, 2020 at 4:13 PM Marco Elver wrote: > > On Tue, Nov 10, 2020 at 11:20PM +0100, Andrey Konovalov wrote: > > The reason cache merging is disabled with KASAN is because KASAN puts its > > metadata right after the allocated object. When the merged caches have > > slightly different sizes, the metadata ends up in different places, which > > KASAN doesn't support. > > > > It might be possible to adjust the metadata allocation algorithm and make > > it friendly to the cache merging code. Instead this change takes a simpler > > approach and allows merging caches when no metadata is present. Which is > > the case for hardware tag-based KASAN with kasan.mode=prod. > > > > Signed-off-by: Andrey Konovalov > > Link: https://linux-review.googlesource.com/id/Ia114847dfb2244f297d2cb82d592bf6a07455dba > > --- > > include/linux/kasan.h | 26 ++++++++++++++++++++++++-- > > mm/kasan/common.c | 11 +++++++++++ > > mm/slab_common.c | 11 ++++++++--- > > 3 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > index 534ab3e2935a..c754eca356f7 100644 > > --- a/include/linux/kasan.h > > +++ b/include/linux/kasan.h > > @@ -81,17 +81,35 @@ struct kasan_cache { > > }; > > > > #ifdef CONFIG_KASAN_HW_TAGS > > + > > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); > > + > > static inline kasan_enabled(void) > > { > > return static_branch_likely(&kasan_flag_enabled); > > } > > -#else > > + > > +slab_flags_t __kasan_never_merge(slab_flags_t flags); > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_enabled()) > > + return __kasan_never_merge(flags); > > + return flags; > > +} > > + > > +#else /* CONFIG_KASAN_HW_TAGS */ > > + > > static inline kasan_enabled(void) > > { > > return true; > > } > > -#endif > > + > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > + > > +#endif /* CONFIG_KASAN_HW_TAGS */ > > > > void __kasan_alloc_pages(struct page *page, unsigned int order); > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) > > @@ -240,6 +258,10 @@ static inline kasan_enabled(void) > > { > > return false; > > } > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} > > static inline void kasan_free_pages(struct page *page, unsigned int order) {} > > static inline void kasan_cache_create(struct kmem_cache *cache, > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > > index 940b42231069..25b18c145b06 100644 > > --- a/mm/kasan/common.c > > +++ b/mm/kasan/common.c > > @@ -81,6 +81,17 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) > > } > > #endif /* CONFIG_KASAN_STACK */ > > > > +/* > > + * Only allow cache merging when stack collection is disabled and no metadata > > + * is present. > > + */ > > +slab_flags_t __kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_stack_collection_enabled()) > > + return flags; > > + return flags & ~SLAB_KASAN; > > +} > > + > > void __kasan_alloc_pages(struct page *page, unsigned int order) > > { > > u8 tag; > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index f1b0c4a22f08..3042ee8ea9ce 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -49,12 +50,16 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work, > > slab_caches_to_rcu_destroy_workfn); > > > > /* > > - * Set of flags that will prevent slab merging > > + * Set of flags that will prevent slab merging. > > + * Use slab_never_merge() instead. > > */ > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > > SLAB_FAILSLAB | SLAB_KASAN) > > Rather than changing this to require using slab_never_merge() which > removes SLAB_KASAN, could we not just have a function > kasan_never_merge() that returns KASAN-specific flags that should never > result in merging -- because as-is now, making kasan_never_merge() > remove the SLAB_KASAN flag seems the wrong way around. > > Could we not just do this: > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > SLAB_FAILSLAB | kasan_never_merge()) > > ?? The issue here was that SLAB_KASAN is defined in slab.h, which includes kasan.h, so we can't have a static inline definition of this function for generic and software tag-based modes. So we can do this, as long as we're fine with having kasan_never_merge() to be an actual function call for all KASAN modes. I guess it's not a problem, so let's do it this way. > > Of course that might be problematic if this always needs to be a > compile-time constant, but currently that's not a requirement. > > > +/* KASAN allows merging in some configurations and will remove SLAB_KASAN. */ > > +#define slab_never_merge() (kasan_never_merge(SLAB_NEVER_MERGE)) > > Braces unnecessary. 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=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=unavailable 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 502F3C5519F for ; Thu, 12 Nov 2020 23:00:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9846020872 for ; Thu, 12 Nov 2020 23:00:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ZA7ZfI8M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9846020872 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 C1F416B005D; Thu, 12 Nov 2020 18:00:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BD22E6B006C; Thu, 12 Nov 2020 18:00:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC13D6B006E; Thu, 12 Nov 2020 18:00:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0007.hostedemail.com [216.40.44.7]) by kanga.kvack.org (Postfix) with ESMTP id 7D8C56B005D for ; Thu, 12 Nov 2020 18:00:33 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 27DF08249980 for ; Thu, 12 Nov 2020 23:00:33 +0000 (UTC) X-FDA: 77477287146.05.grade63_370c40f2730a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 09DE018017FCA for ; Thu, 12 Nov 2020 23:00:33 +0000 (UTC) X-HE-Tag: grade63_370c40f2730a X-Filterd-Recvd-Size: 8910 Received: from mail-pf1-f194.google.com (mail-pf1-f194.google.com [209.85.210.194]) by imf05.hostedemail.com (Postfix) with ESMTP for ; Thu, 12 Nov 2020 23:00:32 +0000 (UTC) Received: by mail-pf1-f194.google.com with SMTP id c66so5943322pfa.4 for ; Thu, 12 Nov 2020 15:00:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=ZA7ZfI8MWSiXS7AZkGgFD1bkrkW1aNC29iy8wrCtuDTNNQ8d4nb6NqqyRWd0LbZJWx PnT0TxZ5Zn4VvWTa/gvoPhVsdEU5qdFGpnGHLSxPwpioP9gBkJdKa5oglMlNfgKkKIkZ MdU4hXgfTfalu5OsqcgO9UuZsFxbWvtfEb8V19U756z01OtQphAnbJ4O407mpKUopo3k uMKOFpjrziWNQQhBG9v4nl5jA1WDrFt+JS+YcbbGSfSFHrrAtdGZUSPNYgVitRv64Gj3 M1YmfLqkYd26omnlUO+84qjxfnzZXpXBbMFpOPAbAY+HnpG5AtH1O2MtzZzZH+SBpAAk byWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=tSXM7gbdZrfBO6GOiL+JHdbnH2b6O5hl35+lHCDFm4FYHvjiW7ZqiyKh0CKrf5jEkV HYjNtfZoNxW1tEM8/FSvM4AVJcfLzLav3wujSCom5vW3rYnwBhjinll6euqSB0qfptaZ Jew6wTNGNX/JufB4A4WDzoV2tPND4Fyh3WM4UAQpMngReAu2BAeFS5GA48IAbP63r6Es rmb8ZG1X5ouGArBWSMToUHjHR/Hni7wUwXo3JPW0j6Y7EVYp0LbYF9mfEGBz6Tad3IWg 8Fu8ldkI2ouugXDFNjxIiAxpMj4R6CLp5j+z9RwPnnj/vwYQZzrXINMT1jMJlTTP5yH2 RQNw== X-Gm-Message-State: AOAM530dPs0yIXfoVXAHgazsZnSMch6JrzR3S5ejmmRa9eMTydQTEb4Y CMcdTAfiz0mqk1kRHhWIrRBZpE2FG8c8VGgAUUbk0A== X-Google-Smtp-Source: ABdhPJzhW5OGR3SOnBOJ/bdTcqYxkR+Ie1nyTBybuHelmvZvjTkLyOkpby7U4aWuzcyUFw24aqfQQRmXqCEi9GgaTBA= X-Received: by 2002:a17:90b:3111:: with SMTP id gc17mr30483pjb.41.1605222031183; Thu, 12 Nov 2020 15:00:31 -0800 (PST) MIME-Version: 1.0 References: <936c0c198145b663e031527c49a6895bd21ac3a0.1605046662.git.andreyknvl@google.com> <20201111151336.GA517454@elver.google.com> In-Reply-To: <20201111151336.GA517454@elver.google.com> From: Andrey Konovalov Date: Fri, 13 Nov 2020 00:00:20 +0100 Message-ID: Subject: Re: [PATCH v2 19/20] kasan, mm: allow cache merging with no metadata To: Marco Elver Cc: Dmitry Vyukov , Alexander Potapenko , Catalin Marinas , Will Deacon , Vincenzo Frascino , Evgenii Stepanov , Andrey Ryabinin , Branislav Rankov , Kevin Brodsky , Andrew Morton , kasan-dev , Linux ARM , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" 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, Nov 11, 2020 at 4:13 PM Marco Elver wrote: > > On Tue, Nov 10, 2020 at 11:20PM +0100, Andrey Konovalov wrote: > > The reason cache merging is disabled with KASAN is because KASAN puts its > > metadata right after the allocated object. When the merged caches have > > slightly different sizes, the metadata ends up in different places, which > > KASAN doesn't support. > > > > It might be possible to adjust the metadata allocation algorithm and make > > it friendly to the cache merging code. Instead this change takes a simpler > > approach and allows merging caches when no metadata is present. Which is > > the case for hardware tag-based KASAN with kasan.mode=prod. > > > > Signed-off-by: Andrey Konovalov > > Link: https://linux-review.googlesource.com/id/Ia114847dfb2244f297d2cb82d592bf6a07455dba > > --- > > include/linux/kasan.h | 26 ++++++++++++++++++++++++-- > > mm/kasan/common.c | 11 +++++++++++ > > mm/slab_common.c | 11 ++++++++--- > > 3 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > index 534ab3e2935a..c754eca356f7 100644 > > --- a/include/linux/kasan.h > > +++ b/include/linux/kasan.h > > @@ -81,17 +81,35 @@ struct kasan_cache { > > }; > > > > #ifdef CONFIG_KASAN_HW_TAGS > > + > > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); > > + > > static inline kasan_enabled(void) > > { > > return static_branch_likely(&kasan_flag_enabled); > > } > > -#else > > + > > +slab_flags_t __kasan_never_merge(slab_flags_t flags); > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_enabled()) > > + return __kasan_never_merge(flags); > > + return flags; > > +} > > + > > +#else /* CONFIG_KASAN_HW_TAGS */ > > + > > static inline kasan_enabled(void) > > { > > return true; > > } > > -#endif > > + > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > + > > +#endif /* CONFIG_KASAN_HW_TAGS */ > > > > void __kasan_alloc_pages(struct page *page, unsigned int order); > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) > > @@ -240,6 +258,10 @@ static inline kasan_enabled(void) > > { > > return false; > > } > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} > > static inline void kasan_free_pages(struct page *page, unsigned int order) {} > > static inline void kasan_cache_create(struct kmem_cache *cache, > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > > index 940b42231069..25b18c145b06 100644 > > --- a/mm/kasan/common.c > > +++ b/mm/kasan/common.c > > @@ -81,6 +81,17 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) > > } > > #endif /* CONFIG_KASAN_STACK */ > > > > +/* > > + * Only allow cache merging when stack collection is disabled and no metadata > > + * is present. > > + */ > > +slab_flags_t __kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_stack_collection_enabled()) > > + return flags; > > + return flags & ~SLAB_KASAN; > > +} > > + > > void __kasan_alloc_pages(struct page *page, unsigned int order) > > { > > u8 tag; > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index f1b0c4a22f08..3042ee8ea9ce 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -49,12 +50,16 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work, > > slab_caches_to_rcu_destroy_workfn); > > > > /* > > - * Set of flags that will prevent slab merging > > + * Set of flags that will prevent slab merging. > > + * Use slab_never_merge() instead. > > */ > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > > SLAB_FAILSLAB | SLAB_KASAN) > > Rather than changing this to require using slab_never_merge() which > removes SLAB_KASAN, could we not just have a function > kasan_never_merge() that returns KASAN-specific flags that should never > result in merging -- because as-is now, making kasan_never_merge() > remove the SLAB_KASAN flag seems the wrong way around. > > Could we not just do this: > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > SLAB_FAILSLAB | kasan_never_merge()) > > ?? The issue here was that SLAB_KASAN is defined in slab.h, which includes kasan.h, so we can't have a static inline definition of this function for generic and software tag-based modes. So we can do this, as long as we're fine with having kasan_never_merge() to be an actual function call for all KASAN modes. I guess it's not a problem, so let's do it this way. > > Of course that might be problematic if this always needs to be a > compile-time constant, but currently that's not a requirement. > > > +/* KASAN allows merging in some configurations and will remove SLAB_KASAN. */ > > +#define slab_never_merge() (kasan_never_merge(SLAB_NEVER_MERGE)) > > Braces unnecessary. 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=-9.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable 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 C58D2C2D0E4 for ; Thu, 12 Nov 2020 23:02:11 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2E73C20797 for ; Thu, 12 Nov 2020 23:02:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UL/2CQIa"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="ZA7ZfI8M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2E73C20797 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kvLFomO054uvVdMkTvjdRhZlQxbC5jScAFvnfNu2xO4=; b=UL/2CQIap+CA7Ri3VOCpkQB+W 5EfMMfpI4rlPlcrQQOM9Qy2D0TCl2HtylbAFK0R068BI/b89hDhaJF/8PoicutoNHYxjQAGCHu7ER tnhMlOdseIzvgePByO/Z3LLcHdXacpUHYakCRanHpBM6sHO2rZ+IUQKxY2KQYpVQHIijZBfYspmRf 0wF79Qe2s8ktCrf3ipFAtcqx9WcBFPm3kGpD9M3yAT62LwijbWY3VDmmt0bb9WO1BUiMwUBRO5Ic5 WF5Awlr5wCWZzZuM7w5eRP+uaa3F0dfy+rJ49jKNb7Q2xBOK08aeDOerv5q8kxCsEOzLA7zkqnYAe 93suHz42w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdLa1-0005Mg-2M; Thu, 12 Nov 2020 23:00:37 +0000 Received: from mail-pf1-x441.google.com ([2607:f8b0:4864:20::441]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdLZx-0005Lf-Gy for linux-arm-kernel@lists.infradead.org; Thu, 12 Nov 2020 23:00:34 +0000 Received: by mail-pf1-x441.google.com with SMTP id b63so2320739pfg.12 for ; Thu, 12 Nov 2020 15:00:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=ZA7ZfI8MWSiXS7AZkGgFD1bkrkW1aNC29iy8wrCtuDTNNQ8d4nb6NqqyRWd0LbZJWx PnT0TxZ5Zn4VvWTa/gvoPhVsdEU5qdFGpnGHLSxPwpioP9gBkJdKa5oglMlNfgKkKIkZ MdU4hXgfTfalu5OsqcgO9UuZsFxbWvtfEb8V19U756z01OtQphAnbJ4O407mpKUopo3k uMKOFpjrziWNQQhBG9v4nl5jA1WDrFt+JS+YcbbGSfSFHrrAtdGZUSPNYgVitRv64Gj3 M1YmfLqkYd26omnlUO+84qjxfnzZXpXBbMFpOPAbAY+HnpG5AtH1O2MtzZzZH+SBpAAk byWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mjN41sX5qjrZFdnqs5FTcpwE8cevolsZVCKbUpaXMpY=; b=D/uYPpKCl7xSU9lq5dqG7pv2HjXObm5uCra/i7gnfPxWP8Pkvdj/Rz5dTp4bMuW6Dd 88qAM556saglAd5MgOVWhD/NfKwDUobNbVTfRhWALRxrb7/CEzl+ss+8TLHeLphIMtWV excN8Hmv5eviE5+04p3mLLOfJeTl3C9zWfhDeH/3Ak+GtZfMOimeJODC1+HgZifCF852 EhFYJoDVRCq8BKfXOet6NqHjl4/qkupSILF6JkmzGrK76AtkMZDQ/1nCRJP23NPFNXG+ 0Sj6KK+SMTStoN1+DVe0VjcaYD8hgr4+s3H50rLWtyu2OBrF2vO7QsJ21qnfMcH4WvyE tr1w== X-Gm-Message-State: AOAM532X/FTSp0hhXsVmTTQ1ukdyD9S4kDaowMm9zogEY3yOp3+0h6uu jRjtENPWN+mA4laIqFbV9gT8jXvbut0xmSRd1+/hUQ== X-Google-Smtp-Source: ABdhPJzhW5OGR3SOnBOJ/bdTcqYxkR+Ie1nyTBybuHelmvZvjTkLyOkpby7U4aWuzcyUFw24aqfQQRmXqCEi9GgaTBA= X-Received: by 2002:a17:90b:3111:: with SMTP id gc17mr30483pjb.41.1605222031183; Thu, 12 Nov 2020 15:00:31 -0800 (PST) MIME-Version: 1.0 References: <936c0c198145b663e031527c49a6895bd21ac3a0.1605046662.git.andreyknvl@google.com> <20201111151336.GA517454@elver.google.com> In-Reply-To: <20201111151336.GA517454@elver.google.com> From: Andrey Konovalov Date: Fri, 13 Nov 2020 00:00:20 +0100 Message-ID: Subject: Re: [PATCH v2 19/20] kasan, mm: allow cache merging with no metadata To: Marco Elver X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201112_180033_626413_B98252E2 X-CRM114-Status: GOOD ( 34.08 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux ARM , Branislav Rankov , Catalin Marinas , Kevin Brodsky , Will Deacon , LKML , kasan-dev , Linux Memory Management List , Alexander Potapenko , Dmitry Vyukov , Andrey Ryabinin , Andrew Morton , Vincenzo Frascino , Evgenii Stepanov Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Nov 11, 2020 at 4:13 PM Marco Elver wrote: > > On Tue, Nov 10, 2020 at 11:20PM +0100, Andrey Konovalov wrote: > > The reason cache merging is disabled with KASAN is because KASAN puts its > > metadata right after the allocated object. When the merged caches have > > slightly different sizes, the metadata ends up in different places, which > > KASAN doesn't support. > > > > It might be possible to adjust the metadata allocation algorithm and make > > it friendly to the cache merging code. Instead this change takes a simpler > > approach and allows merging caches when no metadata is present. Which is > > the case for hardware tag-based KASAN with kasan.mode=prod. > > > > Signed-off-by: Andrey Konovalov > > Link: https://linux-review.googlesource.com/id/Ia114847dfb2244f297d2cb82d592bf6a07455dba > > --- > > include/linux/kasan.h | 26 ++++++++++++++++++++++++-- > > mm/kasan/common.c | 11 +++++++++++ > > mm/slab_common.c | 11 ++++++++--- > > 3 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > index 534ab3e2935a..c754eca356f7 100644 > > --- a/include/linux/kasan.h > > +++ b/include/linux/kasan.h > > @@ -81,17 +81,35 @@ struct kasan_cache { > > }; > > > > #ifdef CONFIG_KASAN_HW_TAGS > > + > > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); > > + > > static inline kasan_enabled(void) > > { > > return static_branch_likely(&kasan_flag_enabled); > > } > > -#else > > + > > +slab_flags_t __kasan_never_merge(slab_flags_t flags); > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_enabled()) > > + return __kasan_never_merge(flags); > > + return flags; > > +} > > + > > +#else /* CONFIG_KASAN_HW_TAGS */ > > + > > static inline kasan_enabled(void) > > { > > return true; > > } > > -#endif > > + > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > + > > +#endif /* CONFIG_KASAN_HW_TAGS */ > > > > void __kasan_alloc_pages(struct page *page, unsigned int order); > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) > > @@ -240,6 +258,10 @@ static inline kasan_enabled(void) > > { > > return false; > > } > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} > > static inline void kasan_free_pages(struct page *page, unsigned int order) {} > > static inline void kasan_cache_create(struct kmem_cache *cache, > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > > index 940b42231069..25b18c145b06 100644 > > --- a/mm/kasan/common.c > > +++ b/mm/kasan/common.c > > @@ -81,6 +81,17 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) > > } > > #endif /* CONFIG_KASAN_STACK */ > > > > +/* > > + * Only allow cache merging when stack collection is disabled and no metadata > > + * is present. > > + */ > > +slab_flags_t __kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_stack_collection_enabled()) > > + return flags; > > + return flags & ~SLAB_KASAN; > > +} > > + > > void __kasan_alloc_pages(struct page *page, unsigned int order) > > { > > u8 tag; > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index f1b0c4a22f08..3042ee8ea9ce 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -49,12 +50,16 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work, > > slab_caches_to_rcu_destroy_workfn); > > > > /* > > - * Set of flags that will prevent slab merging > > + * Set of flags that will prevent slab merging. > > + * Use slab_never_merge() instead. > > */ > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > > SLAB_FAILSLAB | SLAB_KASAN) > > Rather than changing this to require using slab_never_merge() which > removes SLAB_KASAN, could we not just have a function > kasan_never_merge() that returns KASAN-specific flags that should never > result in merging -- because as-is now, making kasan_never_merge() > remove the SLAB_KASAN flag seems the wrong way around. > > Could we not just do this: > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > SLAB_FAILSLAB | kasan_never_merge()) > > ?? The issue here was that SLAB_KASAN is defined in slab.h, which includes kasan.h, so we can't have a static inline definition of this function for generic and software tag-based modes. So we can do this, as long as we're fine with having kasan_never_merge() to be an actual function call for all KASAN modes. I guess it's not a problem, so let's do it this way. > > Of course that might be problematic if this always needs to be a > compile-time constant, but currently that's not a requirement. > > > +/* KASAN allows merging in some configurations and will remove SLAB_KASAN. */ > > +#define slab_never_merge() (kasan_never_merge(SLAB_NEVER_MERGE)) > > Braces unnecessary. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel