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=-5.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 D28D4C433E2 for ; Tue, 8 Sep 2020 15:23:16 +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 79752221F0 for ; Tue, 8 Sep 2020 15:23:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="06PwAjQc"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="HRIGydaP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 79752221F0 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:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=6oh092WwKbW916Hg9aP+4Sn3bfSCP+4EYHP+ohI7iDE=; b=06PwAjQcJZ+FPF4IpH2tVD65B Afa2J5tnCDGkLNZqwctiWx1gep9JClO3l8rHwevecy1WRsZV9WMbMgiIcQaPqkI1b+M8QIznsefSk 8Zd+z1tMNSPD6eQikvvsrLPlR84CPamEGzNEtTFA5JoHfskhgKx06Fm9Z/VMtQk1yOORFbr/37KHp VLjZYfNcieAZWDwi8FhGpM0WCqzBrX/a6rKjPvU1b+uFUUO+XWkSVQXQX+P+pHCu+T5IjGhuzt3TQ BXI+iVgo1lnNZ9sE4lkNRmO9f4L4SjpjGvcuY3Edz2FE9TKGV5Rs8HOctDweqKqo1WpKbrJEnkhOc 8iv+XiMXQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFfRF-0002hD-87; Tue, 08 Sep 2020 15:21:41 +0000 Received: from mail-wm1-x343.google.com ([2a00:1450:4864:20::343]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFfRA-0002eu-EA for linux-arm-kernel@lists.infradead.org; Tue, 08 Sep 2020 15:21:37 +0000 Received: by mail-wm1-x343.google.com with SMTP id z9so17585756wmk.1 for ; Tue, 08 Sep 2020 08:21:35 -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:in-reply-to:user-agent; bh=UKA8pKnO1XgI5TJnprRPT/npi5cBz+avnhKYxtTWBW4=; b=HRIGydaPAJkaOWuBKc7d27QZz482FZ8rdxSZz6uNdbw5Su/m6qKHUczYNAP6sSydlY SNsXYqqSmJdpZTnrP6V1iIozB1fW9Z+0JFH7M56c+ilSEh9sqSauJtmNlSXdyBjohBiB TDbG8PVekF+JFayFi5Z0L8LdKwC2skqUGDXRTStkwl1qX21is4XqFwmAQciUgnoX85F/ B1OM0qfh5OFlBLwkd/XsxUEnoOPnf9+Mnthhd1cS35JdLOOWxo/gfGP7RDKTxFfvOADe Ky0FOihRMcxrl0xU2I5v3tDoPhh8ErdajJR9PSz8TLDBie0V25qN4NN+MhyEr7gk9sgo qH6g== 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:in-reply-to:user-agent; bh=UKA8pKnO1XgI5TJnprRPT/npi5cBz+avnhKYxtTWBW4=; b=q7TNrn4z4HNDQjQTzd543svFS9IHIEvviFCVXXV+FrrWQvozAhOzb3m7j4oRZH22u5 S5H4dCca8A/m6ljmR4sAQDe4P0v5833nO7whhiqqsAaq2RYkzNuElunhElmd1zq2op/t ZX+sunPrf4VAP3tCPbpEm4ntQzR/uVUnypUu+NOHBQTzwzKjo08IXWWiuz3PwSkHdZ3f mhVVgXCLqJlpriJeQkiDdTVV41qZPbk/QJqucqpAnR5P4f1rGkqdq6UvRL/a4YNNnwX2 LVHLMZ7P7ENckTDD32g9WCLhCslecJ8WUtVYVhgbY7HFEo67Hl/hawSggr7jYMz6r4bI b9fw== X-Gm-Message-State: AOAM530CbVHIBOt/+FzRZV0N+Qkrqxp/IZAnGNHp1JltpXfexhKAYcri 1GJ/pZwOjDCyZnYv0ruwJrnl9w== X-Google-Smtp-Source: ABdhPJzR5sEeN+3WkgbqqiMs+xGyY3ppaJbCxHcVgY+v/r3iEbblqpBNU1uJoIblL1gF5vuYTSpBhg== X-Received: by 2002:a1c:7e83:: with SMTP id z125mr96250wmc.32.1599578494903; Tue, 08 Sep 2020 08:21:34 -0700 (PDT) Received: from elver.google.com ([100.105.32.75]) by smtp.gmail.com with ESMTPSA id c10sm31766553wmk.30.2020.09.08.08.21.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Sep 2020 08:21:33 -0700 (PDT) Date: Tue, 8 Sep 2020 17:21:28 +0200 From: Marco Elver To: Vlastimil Babka Subject: Re: [PATCH RFC 00/10] KFENCE: A low-overhead sampling-based memory safety error detector Message-ID: <20200908152128.GA61807@elver.google.com> References: <20200907134055.2878499-1-elver@google.com> <4dc8852a-120d-0835-1dc4-1a91f8391c8a@suse.cz> <1c4a5a6e-1f11-b04f-ebd0-17919ba93bca@suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1c4a5a6e-1f11-b04f-ebd0-17919ba93bca@suse.cz> User-Agent: Mutt/1.14.4 (2020-06-18) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200908_112136_535731_511724A3 X-CRM114-Status: GOOD ( 38.75 ) 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: Mark Rutland , linux-doc@vger.kernel.org, Peter Zijlstra , Catalin Marinas , dave.hansen@linux.intel.com, Linux Memory Management List , Eric Dumazet , Alexander Potapenko , "H. Peter Anvin" , Christoph Lameter , Will Deacon , Jonathan Corbet , the arch/x86 maintainers , kasan-dev , Ingo Molnar , linux-arm-kernel@lists.infradead.org, David Rientjes , Andrey Ryabinin , Kees Cook , paulmck@kernel.org, Jann Horn , Andrey Konovalov , Borislav Petkov , Andy Lutomirski , Thomas Gleixner , Andrew Morton , Dmitriy Vyukov , Greg Kroah-Hartman , LKML , Pekka Enberg , Qian Cai , Joonsoo Kim 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 Tue, Sep 08, 2020 at 04:40PM +0200, Vlastimil Babka wrote: > On 9/8/20 2:16 PM, Alexander Potapenko wrote: > >> Toggling a static branch is AFAIK quite disruptive (PeterZ will probably tell > >> you better), and with the default 100ms sample interval, I'd think it's not good > >> to toggle it so often? Did you measure what performance would you get, if the > >> static key was only for long-term toggling the whole feature on and off (boot > >> time or even runtime), but the decisions "am I in a sample interval right now?" > >> would be normal tests behind this static key? Thanks. > > > > 100ms is the default that we use for testing, but for production it > > should be fine to pick a longer interval (e.g. 1 second or more). > > We haven't noticed any performance impact with neither 100ms nor bigger values. > > Hmm, I see. To add to this, we initially also weren't sure what the results would be toggling the static branches at varying intervals. In the end we were pleasantly surprised, and our benchmarking results always proved there is no noticeable slowdown above 100ms (somewhat noticeable in the range of 1-10ms but it's tolerable if you wanted to go there). I think we were initially, just like you might be, deceived about the time scales here. 100ms is a really long time for a computer. > > Regarding using normal branches, they are quite expensive. > > E.g. at some point we used to have a branch in slab_free() to check > > whether the freed object belonged to KFENCE pool. > > When the pool address was taken from memory, this resulted in some > > non-zero performance penalty. > > Well yeah, if the checks involve extra cache misses, that adds up. But AFAICS > you can't avoid that kind of checks with static key anyway (am I looking right > at is_kfence_address()?) because some kfence-allocated objects will exist even > after the sampling period ended, right? > So AFAICS kfence_alloc() is the only user of the static key and I wonder if it > really makes such difference there. The really important bit here is to differentiate between fast-paths and slow-paths! We insert kfence_alloc() into the allocator fast-paths, which is where the majority of cost would be. On the other hand, the major user of is_kfence_address(), kfence_free(), is only inserted into the slow-path. As a result, is_kfence_address() usage has negligible cost (esp. if the statically allocated pool is used) -- we benchmarked this quite extensively. > > As for enabling the whole feature at runtime, our intention is to let > > the users have it enabled by default, otherwise someone will need to > > tell every machine in the fleet when the feature is to be enabled. > > Sure, but I guess there are tools that make it no difference in effort between 1 > machine and fleet. > > I'll try to explain my general purpose distro-kernel POV. What I like e.g. about > debug_pagealloc and page_owner (and contributed to that state of these features) > is that a distro kernel can be shipped with them compiled in, but they are > static-key disabled thus have no overhead, until a user enables them on boot, > without a need to replace the kernel with a debug one first. Users can enable > them for their own debugging, or when asked by somebody from the distro > assisting with the debugging. > > I think KFENCE has similar potential and could work the same way - compiled in > always, but a static key would eliminate everything, even the > is_kfence_address() checks, [ See my answer for the cost of is_kfence_address() above. In short, until we add is_kfence_address() to fast-paths, introducing yet another static branch would be premature optimization. ] > until it became enabled (but then it would probably > be a one-way street for the rest of the kernel's uptime). Some distro users > would decide to enable it always, some not, but could be advised to when needed. > So the existing static key could be repurposed for this, or if it's really worth > having the current one to control just the sampling period, then there would be two? You can already do this. Just set CONFIG_KFENCE_SAMPLE_INTERVAL=0. When you decide to enable it, set kfence.sample_interval= as a boot parameter. I'll add something to that effect into Documentation/dev-tools/kfence.rst. Thanks, -- Marco _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel