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 3BCD8EB64DA for ; Sun, 16 Jul 2023 15:09:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BA3656B0071; Sun, 16 Jul 2023 11:09:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B53D16B0075; Sun, 16 Jul 2023 11:09:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A1B716B0078; Sun, 16 Jul 2023 11:09:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 938C26B0071 for ; Sun, 16 Jul 2023 11:09:58 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 5E40EA01F2 for ; Sun, 16 Jul 2023 15:09:58 +0000 (UTC) X-FDA: 81017810076.18.6347A55 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf03.hostedemail.com (Postfix) with ESMTP id AEADC2001A for ; Sun, 16 Jul 2023 15:09:56 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf03.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689520196; 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; bh=1LloMWdEhn34E24VfoQLlC+x7Iv0t4aipsyqX5BTZjE=; b=w87uPq+etX8r/TH39nvGSfD8AI48HM2jiJM4Q0rg5YSlxox/QqBZpW0+a23Y9UK7Yq6wBK TlbqfDueNCJQpEW0g4cKA4O3Hvd9UJe2v9hkZyOSTK01OIzE+RnTP5T7/mdLzgUU5or8dv dFXO03yVongh8vYIFYABizlzK79qUx0= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf03.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689520196; a=rsa-sha256; cv=none; b=YhfdnXVw9x1prHfEUvGDyr6dgVoJaUNCP46axEhhliCFZ+6VP1DvMZi5m/CgScxkoifYq9 14WvwfVkfAEO4HyirBtDHXOB84B7tyc/PNIPBjVB66uKv0rfS7Xjd+KbdAiZ8ekcZ3s5Um lIIezBaKfMc6crcxlinGzYhRoFFhzjo= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id C700B60C40; Sun, 16 Jul 2023 15:09:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 098A6C433C7; Sun, 16 Jul 2023 15:09:53 +0000 (UTC) Date: Sun, 16 Jul 2023 08:09:52 -0700 From: Catalin Marinas To: Ryan Roberts Cc: Will Deacon , Ard Biesheuvel , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Anshuman Khandual , Matthew Wilcox , Yu Zhao , Mark Rutland , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v1 11/14] arm64/mm: Wire up PTE_CONT for user mappings Message-ID: References: <20230622144210.2623299-1-ryan.roberts@arm.com> <20230622144210.2623299-12-ryan.roberts@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: AEADC2001A X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: j137zse79texirnxxzw9puq77tcdysaa X-HE-Tag: 1689520196-593090 X-HE-Meta: U2FsdGVkX1/7o0sCJHnaTphGyBkkaZdsK5YDiSl0oSHtfQOKkgMlQio0xiZXzgug57Ba3WWYE95yvHgjZqdtQRRbWI8eX2+diqxmbdWfwKclVPYIwC4QabUWNkQ7y6Iv1MAixgshk58WiUYFFiXkpk02IomHhZOg/hfBf3fkyTnkZ6at7U14lnFaBnIkhZjcn1LTCsqSh8aKVVdeRdpCnOdouPsTMEbcP+bTch/V/MaO/VuyI8ZRVfh+jRojjCuj8aPKwoEB58pFUlTAMJlETUUzLFY/SjO5aYtN6crRKjqVIUu4xm8yHEt0hFbJlJNmmYMFF/4VdhNra6yrpMnGwXgY9MwSnB3YgJeRkkmSuv+TgRt/nRIokvn07dIBYlzRtWclY7ujk24lVECnhx3sGkCDu1gYhnWsP3i93775xGii43gg850wcYp1N3FBGJF9SV1aJ4fRtMQxg+lZQ41uMLWGKgs3wlMo6p5JEY4iXps/5J5f/SBkvieFELGY6Mw3/h6pmG4rePzy3MihKjdreQ48IBzKIoCwDubyrMZo9iQiBzUXVOr7iA1lb514eya6FFCOQw43bhZWLDKVvFAsyUcqA37qDsgKvRM65+DNyF/7m5R5h1XAcPNFP0oZqfOlF6I5SNWotQcXnEcpKKiz4JXiFhgT8JuqofjRX6O9oFNY/ghp2PcPc9OB1xlfKNl6s0eHLO1dPiLt4n2yzNDp8Om/XTLH+S+UhYx7i2nJBJ1lClnZXYF5dnKh0tqSn3dMRG027Z+i/i4wUkgwKFVpXn0L2luSU2ka0cb8dh6KjEX90hi9QxCA6tyiqdz3rj671TDRYzuWVqQhWBBLOKYet1fZsrjKuyVvCLqeeNDZXj/l72GnD7zwg0ukgzmcGPMfDb4aGeDsvm68T/NiNPq5RLdmhBEve1XhhLq3APTEX/5sfhaydxv2nKKfFpKZGPyFZHiC3f+N2G2QZ4bVzyW 7MVihLFN JzVlvD9Xj5e5y+6Q7ApTjbFa3oPHD0WV+cEjrclbJyRw4yRuMkMVZr18T5KSPPMkm21xuGlNzfLBkofMDq2mgKpDQy99mmN06S34hCdky3G8E/i9ZpFeRC1XgnvlbKET9e8+0mdspMfUWObAT5ZeeEjqm1auJ6LHkc7YShq8d779F8JgqcyIURCJufj0C1UyDuETfRXoXcfgZ23XoK2wnD10/saNKoumEy7s7T4d4KHeVbsC4G4UY0RdHk4PmuAzFxk3AUTMpLmtwuQnU6A1M1DrnAZiQmHSLSJD5kgJt9eUargWZ7D7MTjhTlHr6dX84nTVh57do6LvEc97ePRF+FQnSjOfyeqGwxwmZm7dXKGT4siuAl8gLKFjPC1ncDapeZMgtH1FHfm0Ypzg6AxPlK/aNO/ejowW0owqIVViOdNYUogWGJwgdBI5njnFTFzOAe2hCP8FY438Hq92uuwXPLpVw1qN45WKoo6MkMIkl1aByQfYaQCwWdhkcgQS5AlV5WPXlVPiTugeNFqu0j1q6JqFb1Uz1hUsqjGqF 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 Tue, Jul 04, 2023 at 12:09:31PM +0100, Ryan Roberts wrote: > On 03/07/2023 16:17, Catalin Marinas wrote: > > Hi Ryan, > > > > Some comments below. I did not have time to trim down the quoted text, > > so you may need to scroll through it. > > Thanks for the review! > > Looking at the comments, I think they all relate to implementation. Does that > imply that you are happy with the shape/approach? I can't really tell yet as there are a few dependencies and I haven't applied them to look at the bigger picture. My preference would be to handle the large folio breaking/making in the core code via APIs like set_ptes() and eliminate the loop heuristics in the arm64 code to fold/unfold. Maybe it's not entirely possible I need to look at the bigger picture with all the series applied (and on a bigger screen, writing this reply on a laptop in flight). > Talking with Anshuman yesterday, he suggested putting this behind a new Kconfig > option that defaults to disabled and also adding a command line option to > disable it when compiled in. I think that makes sense for now at least to reduce > risk of performance regression? I'm fine with a Kconfig option (maybe expert) but default enabled, otherwise it won't get enough coverage. AFAICT, the biggest risk of regression is the heuristics for folding/unfolding. In general the overhead should be offset by the reduced TLB pressure but we may find some pathological case where this gets in the way. > > On Thu, Jun 22, 2023 at 03:42:06PM +0100, Ryan Roberts wrote: > >> + /* > >> + * No need to flush here; This is always "more permissive" so we > >> + * can only be _adding_ the access or dirty bit. And since the > >> + * tlb can't cache an entry without the AF set and the dirty bit > >> + * is a SW bit, there can be no confusion. For HW access > >> + * management, we technically only need to update the flag on a > >> + * single pte in the range. But for SW access management, we > >> + * need to update all the ptes to prevent extra faults. > >> + */ > > > > On pre-DBM hardware, a PTE_RDONLY entry (writable from the kernel > > perspective but clean) may be cached in the TLB and we do need flushing. > > I don't follow; The Arm ARM says: > > IPNQBP When an Access flag fault is generated, the translation table entry > causing the fault is not cached in a TLB. > > So the entry can only be in the TLB if AF is already 1. And given the dirty bit > is SW, it shouldn't affect the TLB state. And this function promises to only > change the bits so they are more permissive (so AF=0 -> AF=1, D=0 -> D=1). > > So I'm not sure what case you are describing here? The comment for this function states that it sets the access/dirty flags as well as the write permission. Prior to DBM, the page is marked PTE_RDONLY and we take a fault. This function marks the page dirty by setting the software PTE_DIRTY bit (no need to worry) but also clearing PTE_RDONLY so that a subsequent access won't fault again. We do need the TLBI here since PTE_RDONLY is allowed to be cached in the TLB. Sorry, I did not reply to your other comments (we can talk in person in about a week time). I also noticed you figured the above but I had written it already. -- Catalin