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 B4882CFC288 for ; Tue, 15 Oct 2024 11:09:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 49F7A6B0083; Tue, 15 Oct 2024 07:09:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 44FC56B0088; Tue, 15 Oct 2024 07:09:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3178B6B0089; Tue, 15 Oct 2024 07:09:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 103EC6B0083 for ; Tue, 15 Oct 2024 07:09:46 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 221324153E for ; Tue, 15 Oct 2024 11:09:40 +0000 (UTC) X-FDA: 82675565994.07.335C1C9 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf25.hostedemail.com (Postfix) with ESMTP id B24DCA000F for ; Tue, 15 Oct 2024 11:09:38 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=none; spf=pass (imf25.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728990511; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lM+5oTniwkHcvtmkO8BSwiRrPYY1OP6u7tcToruu1Vo=; b=wpxL+u7M7YXm8UaWJZ9DnTpQYxoPNxhkCVQa82eJGIly5biec5CIdNcEe5BHQiva9xTZcz sygO4FNCsnzObKtG0f42LKVp01tiUrXsZaVcd1rVoPE9CUV5D5OnAsD56NqAsecGE452k/ /YW7QP2TH/VdVJ/nidlwHYOuJ2kKSxo= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=none; spf=pass (imf25.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728990511; a=rsa-sha256; cv=none; b=27FjBiQpMJFhNMPSpvlAMzt5NA/4C8+2XMmg3GQR7nKAVrKvNEeqZwwz5qIF0jIrcdKMGN /fhBP2OTSdM3xV9AGItKircoZzC1ZAFS0ZbTQORdLaqmPdEP5vK4udtTpjMd/jNAkcJykX IijLWdbSz3csC4L/2uioCd3W9rwHfPo= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 60B211007; Tue, 15 Oct 2024 04:10:12 -0700 (PDT) Received: from [10.57.86.207] (unknown [10.57.86.207]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DE1ED3F51B; Tue, 15 Oct 2024 04:09:39 -0700 (PDT) Message-ID: Date: Tue, 15 Oct 2024 12:09:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v1 18/57] trace: Remove PAGE_SIZE compile-time constant assumption Content-Language: en-GB To: Steven Rostedt Cc: Andrew Morton , Anshuman Khandual , Ard Biesheuvel , Catalin Marinas , David Hildenbrand , Greg Marsden , Ivan Ivanov , Kalesh Singh , Marc Zyngier , Mark Rutland , Masami Hiramatsu , Matthias Brugger , Miroslav Benes , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org References: <20241014105514.3206191-1-ryan.roberts@arm.com> <20241014105912.3207374-1-ryan.roberts@arm.com> <20241014105912.3207374-18-ryan.roberts@arm.com> <20241014124656.3ffb0f65@gandalf.local.home> From: Ryan Roberts In-Reply-To: <20241014124656.3ffb0f65@gandalf.local.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: B24DCA000F X-Stat-Signature: 845ozy58ira9j378wxstgjkux1etmci1 X-HE-Tag: 1728990578-401419 X-HE-Meta: U2FsdGVkX1+gqB+Yy4zmuhleBtKi+F8RvYyaM8rjYX1QpdKkMQDrWRNSoQL14P1vBzA/Sqb5S3hVYE2jc+mMNOql9Y+du7qSAq+U7rmEcwTq5ynK12KD4mwfZ8QaeMBRod/rsg/Julboj6/79hlrDg4n+wWcncSkq0+sBdQob6JTuVK651H4VvxCrBeckDY+5wst+sFoaryTzKom9P2UIGGv1zcb1ENz+7yv84/KTvBbsXzkXeOtYsv4e8i8dnX4Vve4KFcGEcc57EwP0OQcghAoYx2cRV18nZeqDE+4u0I1VfCzPLLeMEvSxDhpMqZrW2eBJN3fp9bguV8Gn6bEW3sEVbo7j1KCAv/kvWivnfke5iV+9DVp2WKW+O56zUPe8YtcZkE7+fagisfS9gIxWgfkOECvAWvKnVgZgviGCc7z8/cV3PKlQaOg8lG8tXOsUWoTbDNX+7XYyrREZB16I+704XRVRNuM4Fg6rw/u/1dQFXiZakbeU6vzU3Gb1Iw1N0Lghs3nU2+TjCC5y08O5r/4pSGlFkRZ/ztFejZ+km7qoF66oqIyiUJiVfjtpk3rdgSFf2seyag6vjjsmuUhsBBls1KuucPZ9aDP3/1OUtxsBR9lGP0a3RKj7zdcEXhoUH1egiKEw3loBOXREbNAVqYhOfabT1FWdrFA0GxhsE0rZkvZzJbKWVEpBnoNgA1yd64tpor8KlD84bAwrXXSNViRb6QWByAV/pE0eqKo14ECGoRHIOVV+mh4t0rmSyWribU+xmWomIiMZ6RpsBV6KuuzvXbcm6AP3Lc23SKg5g5pK5qiHvi70rHu5J9yX3y7YEmMNVP+LKhYFm0rfCzmCBDs1UDte1f13r7eLXCyZXE3mUuRZ6ss8Alj8Drhah24JkmeDzltx1/P9KSVNQ04a1cSszOJ2dzBySv57+oq2pHQldPM8ogkrzDkNkXug0OBiY6ZQ+EHvHOcc/a/Qh3 QU74c/sQ gpCtYeI9Z9q2QTs6KG1IF9W3UVeo+quW5/GmZ1rqYxiQ1ScP7IocgDpc5JwbiaJkkX/T0TJjr5dSbrrAF/ZSB3Y/cJukc2spsgrOYB1wIklO/pWuyT4oQ8XgXlB5HXjkLY+A2a4ir5bjknzOq1eVDHfobaTyKcRBye14z+d4HjBMeWLAN8raKOaA3emgji71mjAhDXafhRqFdejellkbYkVf30isWK+kWVWFxOi2gAG63+hBWXBDeiU52d9TksJGqvNPBr811GY80SqEeec/nvTOD7Q== 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: List-Subscribe: List-Unsubscribe: On 14/10/2024 17:46, Steven Rostedt wrote: > On Mon, 14 Oct 2024 11:58:25 +0100 > Ryan Roberts wrote: > >> To prepare for supporting boot-time page size selection, refactor code >> to remove assumptions about PAGE_SIZE being compile-time constant. Code >> intended to be equivalent when compile-time page size is active. >> >> Convert BUILD_BUG_ON() BUG_ON() since the argument depends on PAGE_SIZE >> and its not trivial to test against a page size limit. >> >> Redefine FTRACE_KSTACK_ENTRIES so that "struct ftrace_stacks" is always >> sized at 32K for 64-bit and 16K for 32-bit. It was previously defined in >> terms of PAGE_SIZE (and worked out at the quoted sizes for a 4K page >> size). But for 64K pages, the size expanded to 512K. Given the ftrace >> stacks should be invariant to page size, this seemed like a waste. As a >> side effect, it removes the PAGE_SIZE compile-time constant assumption >> from this code. >> >> Signed-off-by: Ryan Roberts >> --- >> >> ***NOTE*** >> Any confused maintainers may want to read the cover note here for context: >> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/ >> >> kernel/trace/fgraph.c | 2 +- >> kernel/trace/trace.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c >> index d7d4fb403f6f0..47aa5c8d8090e 100644 >> --- a/kernel/trace/fgraph.c >> +++ b/kernel/trace/fgraph.c >> @@ -534,7 +534,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, >> if (!current->ret_stack) >> return -EBUSY; >> >> - BUILD_BUG_ON(SHADOW_STACK_SIZE % sizeof(long)); >> + BUG_ON(SHADOW_STACK_SIZE % sizeof(long)); > > Absolutely not! > > BUG_ON() is in no way a substitution of any BUILD_BUG_ON(). BUILD_BUG_ON() > is a non intrusive way to see if something isn't lined up correctly, and > can fix it before you execute any code. BUG_ON() is the most intrusive way > to say something is wrong and you crash the system. Yep, totally agree. I'm afraid this was me being lazy, and there are a couple of other instances where I have done this in other patches that I'll need to fix. Most of the time, I've been able to keep BUILD_BUG_ON() and simply compare against a page size limit. Looking at this again, perhaps the better solution is to define SHADOW_STACK_SIZE as PAGE_SIZE_MIN? Then it remains a compile-time constant. Is there any need for SHADOW_STACK_SIZE to increase with page size? > > Not to mention, when function graph tracing is enabled, this gets triggered > for *every* function call! So I do not want any runtime test done. Every > nanosecond counts in this code path. > > If anything, this needs to be moved to initialization and checked once, if > it fails, gives a WARN_ON() and disables function graph tracing. I'm hoping my suggestion above to decouple SHADOW_STACK_SIZE from PAGE_SIZE is acceptable and simpler? If not, happy to do as you suggest here. Thanks, Ryan > > -- Steve > > >> >> /* Set val to "reserved" with the delta to the new fgraph frame */ >> val = (FGRAPH_TYPE_RESERVED << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET; >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c >> index c3b2c7dfadef1..0f2ec3d30579f 100644 >> --- a/kernel/trace/trace.c >> +++ b/kernel/trace/trace.c >> @@ -2887,7 +2887,7 @@ trace_function(struct trace_array *tr, unsigned long ip, unsigned long >> /* Allow 4 levels of nesting: normal, softirq, irq, NMI */ >> #define FTRACE_KSTACK_NESTING 4 >> >> -#define FTRACE_KSTACK_ENTRIES (PAGE_SIZE / FTRACE_KSTACK_NESTING) >> +#define FTRACE_KSTACK_ENTRIES (SZ_4K / FTRACE_KSTACK_NESTING) >> >> struct ftrace_stack { >> unsigned long calls[FTRACE_KSTACK_ENTRIES]; >