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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E0A79C4345F for ; Fri, 12 Apr 2024 09:25:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=OI+Q2G8JSfgnQg0Jsl1YAM30LY81he04Ke3JI6OYdH8=; b=GTdx1PjcSB/62B 4v27SQa3CKRLi8iRPRRxDQxmBeKYslsH1FVAaDe2fR7f4/PVclfoAhnqHIWzXr7UJwF/puxaJH6Xq bORP59qa2RwtSNZWenyACekyaVD7bc0oYcr9rTQODuVU8K99QDE2Pq4pXcnR0dvYALCCZJ/+OVolP 2UPzHPuIfuI2kPsxEl2D/u/7g1lbohPUdQCEfjMsP+/S5+enBoS3JPhte+xj8z3OKkJzc8+/u0kK9 JYJoosL6eeTH38/dPr/MhNd9C3yzfyrT2o82ekUHu6/NzFNFUYTcewQLieoKDC9Na2gL578LUMSCh h2uN3grCVWMmFDBfBCnQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvD9z-0000000GRI3-1t4l; Fri, 12 Apr 2024 09:25:27 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvD9w-0000000GRGH-3LKc for linux-arm-kernel@lists.infradead.org; Fri, 12 Apr 2024 09:25:26 +0000 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 A520E339; Fri, 12 Apr 2024 02:25:50 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.17.205]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A61953F64C; Fri, 12 Apr 2024 02:25:19 -0700 (PDT) Date: Fri, 12 Apr 2024 10:25:16 +0100 From: Mark Rutland To: Ryan Roberts Cc: Catalin Marinas , Will Deacon , Ard Biesheuvel , David Hildenbrand , Donald Dutile , Eric Chanudet , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Itaru Kitayama Subject: Re: [PATCH v2 3/4] arm64: mm: Don't remap pgtables for allocate vs populate Message-ID: References: <20240404143308.2224141-1-ryan.roberts@arm.com> <20240404143308.2224141-4-ryan.roberts@arm.com> <37336367-f876-4429-a8a6-f887fc7f69ee@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <37336367-f876-4429-a8a6-f887fc7f69ee@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240412_022525_011513_7357E167 X-CRM114-Status: GOOD ( 24.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Fri, Apr 12, 2024 at 08:53:18AM +0100, Ryan Roberts wrote: > Hi Mark, > > [...] > > > Does something like the below look ok to you? The trade-off performance-wise is > > that late uses will still use the fixmap, and will redundantly zero the tables, > > but the logic remains fairly simple, and I suspect the overhead for late > > allocations might not matter since the bulk of late changes are non-allocating. > > @@ -303,12 +301,18 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr, > > pudval |= PUD_TABLE_PXN; > > BUG_ON(!pgtable_alloc); > > pmd_phys = pgtable_alloc(PMD_SHIFT); > > + > > + pmdp = pmd_set_fixmap(pmd_phys); > > + init_clear_pgtable(pmdp); > > + > > __pud_populate(pudp, pmd_phys, pudval); > > pud = READ_ONCE(*pudp); > > + } else { > > + pmdp = pmd_set_fixmap(pud_page_paddr(pud)); > > } > > BUG_ON(pud_bad(pud)); > > > > - pmdp = pmd_set_fixmap_offset(pudp, addr); > > + pmdp += pmd_index(addr); > > do { > > pgprot_t __prot = prot; > > > > @@ -345,12 +349,18 @@ static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end, > > p4dval |= P4D_TABLE_PXN; > > BUG_ON(!pgtable_alloc); > > pud_phys = pgtable_alloc(PUD_SHIFT); > > + > > + pudp = pud_set_fixmap(pud_phys); > > + init_clear_pgtable(pudp); > > + > > __p4d_populate(p4dp, pud_phys, p4dval); > > p4d = READ_ONCE(*p4dp); > > + } else { > > + pudp = pud_set_fixmap(p4d_page_paddr(p4d)); > > With this change I end up in pgtable folding hell. pXX_set_fixmap() is defined > as NULL when the level is folded (and pXX_page_paddr() is not defined at all). > So it all compiles, but doesn't boot. Sorry about that; I had not thought to check the folding logic when hacking that up. > I think the simplest approach is to follow this pattern: > > ----8<---- > @@ -340,12 +338,15 @@ static void alloc_init_pud(p4d_t *p4dp, unsigned long > addr, unsigned long end, > p4dval |= P4D_TABLE_PXN; > BUG_ON(!pgtable_alloc); > pud_phys = pgtable_alloc(PUD_SHIFT); > + pudp = pud_set_fixmap(pud_phys); > + init_clear_pgtable(pudp); > + pudp += pud_index(addr); > __p4d_populate(p4dp, pud_phys, p4dval); > - p4d = READ_ONCE(*p4dp); > + } else { > + BUG_ON(p4d_bad(p4d)); > + pudp = pud_set_fixmap_offset(p4dp, addr); > } > - BUG_ON(p4d_bad(p4d)); > > - pudp = pud_set_fixmap_offset(p4dp, addr); > do { > pud_t old_pud = READ_ONCE(*pudp); > ----8<---- > > For the map case, we continue to use pud_set_fixmap_offset() which is always > defined (and always works correctly). > > Note also that the previously unconditional BUG_ON needs to be prior to the > fixmap call to be useful, and its really only valuable in the map case because > for the alloc case we are the ones setting the p4d so we already know its not > bad. This means we don't need the READ_ONCE() in the alloc case. > > Shout if you disagree. That looks good, and I agree with the reasoning here. Thanks for working on this! Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel