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 C2D6AC4345F for ; Thu, 11 Apr 2024 14:49:07 +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=3OxF+62Ux0Ey/p3Rj21kMKm66+FquL+NedY3Dnj8Cqk=; b=ssIyiTfuwR6U6c 8bKZkL/Z9MWNv+gQU1U6I70hEJeDRR2tK4uy7xrruIE4VjnGxCMOIY8bRjFg1WnUFuDT9gqYbMiWQ qL3ZbvKKU0HQ1aPpb3Wxu1jom1mRsXV+aSHMIoRKHmGTDfN0AjfeA+C8FcX7Jf3vsCjLnwAQXWH9P D0aMBlPgzXLZ+RKGGX9e2lxvnK79M+TTRA+OABiNziYMWI2D9TIC/i7yludY7I69psLb1r68tMrNh XkfteX7FAx63G3PZ7bq1sjU3rCHoQHXnvL3tlmagyRCrWw/YhSj3fcZnA/mJtIPK/raY9owULZf/v Aus0E++1NRd1IP+6ZcOQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1ruvjR-0000000CZ3u-1NUN; Thu, 11 Apr 2024 14:48:53 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1ruvjI-0000000CYwI-3ryG for linux-arm-kernel@bombadil.infradead.org; Thu, 11 Apr 2024 14:48:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=xnhMx/HmuIQsAEZtb8QcAvufOwpBTV8oPuKQJvrhbjc=; b=JCF9iKLBRGu25bLn/Prn+W2X4i HJv1R4GyIpeQ8QCNMhpj7hu+QaBNpYrbSrDgL0V7mwO5MoJsA92wO6Dq/w2nuRvU1g2yPURgG4Wmq ngglXzjBI/D9xlafv4rsENNjnGJDJHbjN/CTFoq926PE/bks/IU0w7YD6DcJV/icTwiYGwTfmF7MU VUs1PXEPnvfeBXm3LDroM72Swh6Vd+tIs988tmqVgQyIjRqLtZAC6a6N8mUX9T4C33VMMeDy7hGIv GODlf6fs1qkkNLqJJn5CEZTsXDgfyKltrj3QFuYG1Ifu2p+TqGgf/xBJJU8VUoWZk16cjROIUWrWL 7lY32Vqg==; Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1ruvjD-00000008fXp-3sj6 for linux-arm-kernel@lists.infradead.org; Thu, 11 Apr 2024 14:48:43 +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 23C6B339; Thu, 11 Apr 2024 07:49:05 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.16.129]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D13E93F64C; Thu, 11 Apr 2024 07:48:33 -0700 (PDT) Date: Thu, 11 Apr 2024 15:48:24 +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> <37d4c278-3780-49ce-bd7e-e8f2ff4501fd@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <37d4c278-3780-49ce-bd7e-e8f2ff4501fd@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240411_154840_490750_03807E4E X-CRM114-Status: GOOD ( 43.07 ) 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 Thu, Apr 11, 2024 at 02:37:49PM +0100, Ryan Roberts wrote: > On 11/04/2024 14:02, Mark Rutland wrote: > > On Thu, Apr 04, 2024 at 03:33:07PM +0100, Ryan Roberts wrote: > >> During linear map pgtable creation, each pgtable is fixmapped / > >> fixunmapped twice; once during allocation to zero the memory, and a > >> again during population to write the entries. This means each table has > >> 2 TLB invalidations issued against it. Let's fix this so that each table > >> is only fixmapped/fixunmapped once, halving the number of TLBIs, and > >> improving performance. > >> > >> Achieve this by abstracting pgtable allocate, map and unmap operations > >> out of the main pgtable population loop code and into a `struct > >> pgtable_ops` function pointer structure. This allows us to formalize the > >> semantics of "alloc" to mean "alloc and map", requiring an "unmap" when > >> finished. So "map" is only performed (and also matched by "unmap") if > >> the pgtable has already been allocated. > >> > >> As a side effect of this refactoring, we no longer need to use the > >> fixmap at all once pages have been mapped in the linear map because > >> their "map" operation can simply do a __va() translation. So with this > >> change, we are down to 1 TLBI per table when doing early pgtable > >> manipulations, and 0 TLBIs when doing late pgtable manipulations. > >> > >> Execution time of map_mem(), which creates the kernel linear map page > >> tables, was measured on different machines with different RAM configs: > >> > >> | Apple M2 VM | Ampere Altra| Ampere Altra| Ampere Altra > >> | VM, 16G | VM, 64G | VM, 256G | Metal, 512G > >> ---------------|-------------|-------------|-------------|------------- > >> | ms (%) | ms (%) | ms (%) | ms (%) > >> ---------------|-------------|-------------|-------------|------------- > >> before | 13 (0%) | 162 (0%) | 655 (0%) | 1656 (0%) > >> after | 11 (-15%) | 109 (-33%) | 449 (-31%) | 1257 (-24%) > > > > Do we know how much of that gain is due to the early pgtable creation doing > > fewer fixmap/fixunmap ops vs the later operations using the linear map? > > > > I suspect that the bulk of that is down to the early pgtable creation, and if > > so I think that we can get most of the benefit with a simpler change (see > > below). > > All of this improvement is due to early pgtable creation doing fewer > fixmap/fixunmaps; I'm only measuring the execution time of map_mem(), which only > uses the early ops. > > I haven't even looked to see if there are any hot paths where the late ops > benefit. I just saw it as a happy side-effect. Ah, of course. I skimmed this and forgot this was just timing map_mem(). [...] > > There's a lot of boilerplate that results from having the TYPE_Pxx enumeration > > and needing to handle that in the callbacks, and it's somewhat unfortunate that > > the callbacks can't use the enum type directly (becuase the KPTI allocator is > > in another file). > > > > I'm not too keen on all of that. > > Yes, I agree its quite a big change. And all the switches are naff. But I > couldn't see a way to avoid it and still get all the "benefits". > > > As above, I suspect that most of the benefit comes from minimizing the > > map/unmap calls in the early table creation, and I think that we can do that > > without needing all this infrastructure if we keep the fixmapping explciit > > in the alloc_init_pXX() functions, but factor that out of > > early_pgtable_alloc(). > > > > Does something like the below look ok to you? > > Yes this is actually quite similar to my first attempt, but then I realised I > could get rid of the redudancies too. > > > The trade-off performance-wise is > > that late uses will still use the fixmap, and will redundantly zero the tables, > > I think we can mitigate the redudant zeroing for most kernel configs; tell the > allocator we don't need it to be zeroed. There are some obscure configs where > pages are zeroed on free instead of on alloc IIRC, so those would still have a > redundant clear but they are not widely used AIUI. (see bleow). That sounds fine to me; minor comment below. > > 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. > > Its just the fixmap overhead that remains... True; my thinking there is that almost all of the later changes are for smaller ranges than the linear map (~10s of MB vs GBs in your test data), so I'd expect the overhead of those to be dominated by the cost of mappin the linear map. The only big exception is arch_add_memory(), but memory hotplug is incredibly rare, and we're not making it massively slower than it already was... > I'll benchmark with your below change, and also have a deeper look to check if > there are real places where fixmap might cause slowness for late ops. Thanks! [...] > > @@ -475,8 +491,6 @@ static phys_addr_t __pgd_pgtable_alloc(int shift) > > void *ptr = (void *)__get_free_page(GFP_PGTABLE_KERNEL); > > How about: > > void *ptr = (void *)__get_free_page(GFP_PGTABLE_KERNEL & ~__GFP_ZERO); Looks good to me, assuming we add a comment to say it'll be zeroed in init_clear_pgtable(). Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel