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 0A23DC48BEB for ; Fri, 16 Feb 2024 10:59:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5AD9B8D0006; Fri, 16 Feb 2024 05:59:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 55A4E8D0001; Fri, 16 Feb 2024 05:59:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3FC198D0006; Fri, 16 Feb 2024 05:59:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 2B6F88D0001 for ; Fri, 16 Feb 2024 05:59:32 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id C4FD1804F4 for ; Fri, 16 Feb 2024 10:59:31 +0000 (UTC) X-FDA: 81797370942.07.7625544 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf10.hostedemail.com (Postfix) with ESMTP id EB912C0005 for ; Fri, 16 Feb 2024 10:59:29 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf10.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708081170; 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=/OibmKVoR1wjhGp+5tz/tQoK3udLn3xyN5pNVm9XbIs=; b=XjBbBX14+bU6jq19hZX5R/obitjoKdo75joGxf8ltoGLbgBODymreFny35DYCcbUWYu1g0 jVgD1j3n+E5nDtukcN7nm9aeM/Ij3XdNfqggtztpEIhqJcifT1B1VizO+FlRKACJZjSfP5 qbHTaOMI8Wo8sCfEASTmk6OpXBvgNjs= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf10.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708081170; a=rsa-sha256; cv=none; b=Etpu5fJIWinU9PpHjoYKGqrnD2NNxLHEXH/G1JE/PiI6Ze3eAHXxhfTQGOB+UzIMb0gE9+ g46WJ/vXE7kVTllXvQRptw4Re6trqoQ9B0Pshy9nR6WH3PZM8UAGT6RxOSfvh4ZLU7GvWk GOok4HDyO9S13batK1cLPgXgHlaLdBc= 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 910E11FB; Fri, 16 Feb 2024 03:00:09 -0800 (PST) Received: from [192.168.68.110] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E2F263F766; Fri, 16 Feb 2024 02:59:26 -0800 (PST) Message-ID: Date: Fri, 16 Feb 2024 10:59:25 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] mm/filemap: Allow arch to request folio size for exec memory Content-Language: en-GB To: Andrew Morton Cc: Catalin Marinas , Will Deacon , Mark Rutland , "Matthew Wilcox (Oracle)" , David Hildenbrand , Barry Song <21cnbao@gmail.com>, John Hubbard , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org References: <20240215154059.2863126-1-ryan.roberts@arm.com> <20240215144849.aba06863acc08b8ded09a187@linux-foundation.org> From: Ryan Roberts In-Reply-To: <20240215144849.aba06863acc08b8ded09a187@linux-foundation.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: d5sau1z17u15uwpag5ghsouzebo9z9qx X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: EB912C0005 X-HE-Tag: 1708081169-53368 X-HE-Meta: U2FsdGVkX19cLMVJA7n78dpvxkgNecZrYd1yG5MrW2fW7WRLvBiXkQjNVIn/Ig2B2W89yxRHvfSvjw5/Dx3m9Lhz7Al1jUrDVPLLx3Xz2O9AAQN0+HO9YQUtddou2q3vNo7bXB0RT7CZzAm8gVYMT5+AovCJSM6kVAcyHQOoJCdx7flKMo0REGKy7PUW+3C6Bff5hZQ+KyUtDOX4VUN6j1httnT/DcRdPno5AOqJS4DBgq+bvDHXntVAZsBWw+ce+VMinXQeFcS8qtz6g7DGq4jWsEbJfwJAKhP/XOakmZuJxX3kwJsL8t693xp0Y6qbj3CHY6SPgv0bzClYUes44+l5zo5ecPuCtmLJlRV/ZrXaj1Ie9hBzBZYSN6zpknN9cL6fFq09Y07X0OR6MablSJZ6Ci7oLZpmKPXc1pNq/jwFsGITNjuxR5NMj8tUxqA3iJWbb5bTjefVT6FV/RgkGUkeYr4W66DR/3TiJLa0K0I6YQNb+UrlRGdJSFgRjrcubdBrpLj/piALJpGhtBF9+SrSiolL9EMlFsNukSbEsihWFSu5c3Y2eBbtDar3vKv7QLTy5Vz1UL5+B3zSAu3xTR4HXkPzbaBcvilinNiePyKoPyUJFNcPSLLiTTT4p5g7twgLI+1LpqRjMoPfJCbYYSfSzsSKHoCeTx1PBhR2vl8l3WNn9qLNSnGOVWxlerX96sPDY9m8Rr98DqIwoK4d8+aGX42wnBXMzhJhtmr+llVaK/OvHUengzvxzMlKhYf2Ievytc8uTY6xhPOKgUC88TkVtPxvATjtmUUv9s/lZoqyRqjmBAnszC/l7YjlIq5+O3D4FCEEHCsK6aTO1Nii8RjHaVTEuwAB 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 15/02/2024 22:48, Andrew Morton wrote: > On Thu, 15 Feb 2024 15:40:59 +0000 Ryan Roberts wrote: > >> Change the readahead config so that if it is being requested for an >> executable mapping, do a synchronous read of an arch-specified size in a >> naturally aligned manner. > > Some nits: Thanks for taking a look, Andrew! > >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -1115,6 +1115,18 @@ static inline void update_mmu_cache_range(struct vm_fault *vmf, >> */ >> #define arch_wants_old_prefaulted_pte cpu_has_hw_af >> >> +/* >> + * Request exec memory is read into pagecache in at least 64K folios. The >> + * trade-off here is performance improvement due to storing translations more >> + * effciently in the iTLB vs the potential for read amplification due to reading > > "efficiently" ACK; will fix if there is a v3 > >> + * data from disk that won't be used. The latter is independent of base page >> + * size, so we set a page-size independent block size of 64K. This size can be >> + * contpte-mapped when 4K base pages are in use (16 pages into 1 iTLB entry), >> + * and HPA can coalesce it (4 pages into 1 TLB entry) when 16K base pages are in >> + * use. >> + */ >> +#define arch_wants_exec_folio_order() ilog2(SZ_64K >> PAGE_SHIFT) >> + > > To my eye, "arch_wants_foo" and "arch_want_foo" are booleans. Either > this arch wants a particular treatment or it does not want it. > > I suggest a better name would be "arch_exec_folio_order". ACK; will fix if there is a v3 > >> static inline bool pud_sect_supported(void) >> { >> return PAGE_SIZE == SZ_4K; >> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h >> index aab227e12493..6cdd145cbbb9 100644 >> --- a/include/linux/pgtable.h >> +++ b/include/linux/pgtable.h >> @@ -407,6 +407,18 @@ static inline bool arch_has_hw_pte_young(void) >> } >> #endif >> >> +#ifndef arch_wants_exec_folio_order >> +/* >> + * Returns preferred minimum folio order for executable file-backed memory. Must >> + * be in range [0, PMD_ORDER]. Negative value implies that the HW has no >> + * preference and mm will not special-case executable memory in the pagecache. >> + */ > > I think this comment contains material which would be useful above the > other arch_wants_exec_folio_order() implementation - the "must be in > range" part. So I suggest all this material be incorporated into a > single comment which describes arch_wants_exec_folio_order(). Then > this comment can be removed entirely. Assume the reader knows to go > seek the other definition for the commentary. Hmm... The approach I've been taking for other arch-overridable helpers is to put the API spec against the default implementation (i.e. here) then put comments about the specific implementation against the override. If anything I would prefer to formalize this comment into proper doc header comment and leave it here (see for example set_ptes(), and in recent patches now in mm-unstable; get_and_clear_full_ptes(), wrprotect_ptes(), etc). I'll move all of this to the arm64 code if you really think that's the right approach, but that's not my personal preference. Thanks, Ryan > >> +static inline int arch_wants_exec_folio_order(void) >> +{ >> + return -1; >> +} >> +#endif >> + >> #ifndef arch_check_zapped_pte >> static inline void arch_check_zapped_pte(struct vm_area_struct *vma, >> pte_t pte) >> diff --git a/mm/filemap.c b/mm/filemap.c >> index 142864338ca4..7954274de11c 100644 >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -3118,6 +3118,25 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf) >> } >> #endif >> >> + /* >> + * Allow arch to request a preferred minimum folio order for executable >> + * memory. This can often be beneficial to performance if (e.g.) arm64 >> + * can contpte-map the folio. Executable memory rarely benefits from >> + * read-ahead anyway, due to its random access nature. > > "readahead" > >> + */ >> + if (vm_flags & VM_EXEC) { >> + int order = arch_wants_exec_folio_order(); >> + >> + if (order >= 0) { >> + fpin = maybe_unlock_mmap_for_io(vmf, fpin); >> + ra->size = 1UL << order; >> + ra->async_size = 0; >> + ractl._index &= ~((unsigned long)ra->size - 1); >> + page_cache_ra_order(&ractl, ra, order); >> + return fpin; >> + } >> + } >> + >> /* If we don't want any read-ahead, don't bother */ >> if (vm_flags & VM_RAND_READ) >> return fpin; >