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 3BD1CD6B6BC for ; Wed, 30 Oct 2024 18:30:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 540A76B00AE; Wed, 30 Oct 2024 14:30:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F1066B00B2; Wed, 30 Oct 2024 14:30:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3B8A46B00B3; Wed, 30 Oct 2024 14:30:33 -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 1E7F56B00AE for ; Wed, 30 Oct 2024 14:30:33 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9425EC0F29 for ; Wed, 30 Oct 2024 18:30:32 +0000 (UTC) X-FDA: 82731108852.28.6BDFE44 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf23.hostedemail.com (Postfix) with ESMTP id CC26F140010 for ; Wed, 30 Oct 2024 18:30:13 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf23.hostedemail.com: domain of cmarinas@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730312912; a=rsa-sha256; cv=none; b=xHGfZTUjXbH7/BMc9y6MG6IOo+2ecP4OOpiJdKGHoOG+rj22n4MrGruoeFAODiYsvyDJcL 0OzweGuZWETJiyQopGF0YNHOt5vU9k9gR5dcXUDGxZgm0qGZDJOxF6EjF94xQkjTc7P2Nn 3nuXP1P1HrCk9SHeTsq8tZ5H4w2UwCc= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf23.hostedemail.com: domain of cmarinas@kernel.org designates 147.75.193.91 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=1730312912; 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=thw1+1OfbQud0IaCets3o53FyKp+7ytUT2WEj5xYAW8=; b=E+G+OK/nRrfwYUJutRGTMz3aheQj/iwWQGOiIFYalPyl3ArDAm6KexiOvHc3QqRaoeqTf6 /AYJVUlE8HP/1zW/BCy+o8XPpHFEh79lMzTFq0HPFRWwwA7bGdTUnnedOi2nfh5LPUKuop LP1Zcn25PhxJdVnooLwbqekUtlwnW+k= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 197B8A42EE3; Wed, 30 Oct 2024 18:28:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 948D2C4CECE; Wed, 30 Oct 2024 18:30:26 +0000 (UTC) Date: Wed, 30 Oct 2024 18:30:24 +0000 From: Catalin Marinas To: Vlastimil Babka Cc: Lorenzo Stoakes , Andrew Morton , "Liam R . Howlett" , Jann Horn , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Linus Torvalds , Peter Xu , Will Deacon , Mark Brown , "David S . Miller" , Andreas Larsson , "James E . J . Bottomley" , Helge Deller , Yang Shi Subject: Re: [PATCH hotfix 6.12 v4 4/5] mm: refactor arch_calc_vm_flag_bits() and arm64 MTE handling Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: CC26F140010 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: oqjc9xaeyuyacsw85rn9ay3gzfonbrnp X-HE-Tag: 1730313013-607830 X-HE-Meta: U2FsdGVkX19+1MBEznglLMjsCZOPY8591Ns8CLlHG5TMZMeYU9WCx/oQ6ImpHh3z+m16a22VFr9OtB3mAPqWLosvtotIion3hBdruLffcpaYgabkJYHrNfxzG9avt5QClYJc1Ajcfg7pVvp5+Xc8v+I/wpv+xbMkcpY4c/dg6Tpn0qzxJSL5FVWnDWd0cRk6J5Xf/a9j2aRka16M6IHTzFpE1niiGyNnn0nsv6wHXm0Xa9LtLKYz0LqoKx/S43AirYM2kl5BIMfWTLiXQWqBwMbPL7ujsuqn7SJoZZc9+w8FVJt59WPX99qkKJoXpV24WUCqw/NyPxI6neKtWDlDaG58a8gl3+bA66ZTubLYuFY6E7VortypWljI21jD3GCVZwYbdsakG3Fd2WpgNQbegLAnXXzRjv1bljAKTyp6yr0junSuKVf0PWCKJl9A7VSTeouAYJ6wEB8bLsC6509spUmGii/wnglwhgsxvfBqhzcLi+F4FZq+D27+Q2xrfhjqSm8nFZgriUIpe38rmTiG+SInAQVSapd1bqrHqN586qu4QvoZZmDwUJNx7ixw+cu4C4rNRacCzqIOELFXxb/MzuIXiwP2qY1hyCrPLRKRFnpO/McV7vtZjmsSm6FvM+nz+6Do1qKPeA9O9wLOGW70mPpvzFKfe6TW7Bq2WI5bIa7rBQ1JIJWOyZPZketzat3gVhnVpdsMUztrIyTmdPNIv8ONRMjeCChXMf9E84VWRY0dWq3xwa4dTzTbmPJG5TgdCDsKftwjN+TcPwkzF/1W6VTKQQvNaZnf4WpSZKCm4Jay3Xx644+dfkQCrF26GNBxwjcdbWPxKTGxEb0rAIVDCPDeQAphUKfQfrpjsFrvMwaWLOZe0K6JCkJ5u/THVGUcndGMjr9P1ierH/G20sBvm7aNaO7yBs8S1GmAr48tv828WirMQMQqgnF4P/+N4010ZSi/b+/0B5x1hEM3xVB s+FE6/Q1 5i4KHZVhpIZNEXnkxFvl3kZ5e5pekj3ZTICxbQezGqYVjMpbSnZYxp1jH9baUcJleaw8QJGLtZ1L/f1aGM0ONSAdEuFyLsB+sYd7tOO8F8GiWxnYjvQfZkWhBQ4MMz9y+Ko4QihjI5QLhNKJF30pAX4VT6FCxFgPNkTFV/ZWptYlzVNFtLKpS/esFe0IbqDL0AbiDd3KCdSVIsDQ2uzDAw2HP972+g6ZcNGZ9d9Mire9Pi9Lw1bEmFRvlwo325TlxSherQ141ScldsF+OBH0NrB2LhhdhO19W1/T3U79CtNOJVcfFxzPp7T8F+RJWwMBH9d0o2qNlXkjzJwjZRRkL8u0P34afiMYrGEphqxo7Yoh6FrH7K5Py1ffQW2OCTsaDUyUzlzxfTW07YCw= 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 Wed, Oct 30, 2024 at 12:09:43PM +0100, Vlastimil Babka wrote: > On 10/30/24 11:58, Catalin Marinas wrote: > > On Wed, Oct 30, 2024 at 10:18:27AM +0100, Vlastimil Babka wrote: > >> On 10/29/24 19:11, Lorenzo Stoakes wrote: > >> > --- a/arch/arm64/include/asm/mman.h > >> > +++ b/arch/arm64/include/asm/mman.h > >> > @@ -6,6 +6,8 @@ > >> > > >> > #ifndef BUILD_VDSO > >> > #include > >> > +#include > >> > +#include > >> > #include > >> > > >> > static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, > >> > @@ -31,19 +33,21 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, > >> > } > >> > #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey) > >> > > >> > -static inline unsigned long arch_calc_vm_flag_bits(unsigned long flags) > >> > +static inline unsigned long arch_calc_vm_flag_bits(struct file *file, > >> > + unsigned long flags) > >> > { > >> > /* > >> > * Only allow MTE on anonymous mappings as these are guaranteed to be > >> > * backed by tags-capable memory. The vm_flags may be overridden by a > >> > * filesystem supporting MTE (RAM-based). > >> > >> We should also eventually remove the last sentence or even replace it with > >> its negation, or somebody might try reintroducing the pattern that won't > >> work anymore (wasn't there such a hugetlbfs thing in -next?). > > > > I agree, we should update this comment as well though as a fix this > > patch is fine for now. > > > > There is indeed a hugetlbfs change in -next adding VM_MTE_ALLOWED. It > > should still work after the above change but we'd need to move it over > > I guess it will work after the above change, but not after 5/5? > > > here (and fix the comment at the same time). We'll probably do it around > > -rc1 or maybe earlier once this fix hits mainline. > > I assume this will hopefully go to rc7. > > > I don't think we have > > an equivalent of shmem_file() for hugetlbfs, we'll need to figure > > something out. > > I've found is_file_hugepages(), could work? And while adding the hugetlbfs > change here, the comment could be adjusted too, right? Right, thanks for the hint. I guess the conflict resolution in -next will be something like: ----------------8<---------------------------------- diff --cc arch/arm64/include/asm/mman.h index 798d965760d4,65bc2b07f666..8b9b819196e5 --- a/arch/arm64/include/asm/mman.h +++ b/arch/arm64/include/asm/mman.h @@@ -42,7 -39,7 +42,7 @@@ static inline unsigned long arch_calc_v * filesystem supporting MTE (RAM-based). */ if (system_supports_mte() && - ((flags & MAP_ANONYMOUS) || shmem_file(file))) - (flags & (MAP_ANONYMOUS | MAP_HUGETLB))) ++ ((flags & (MAP_ANONYMOUS | MAP_HUGETLB)) || shmem_file(file))) return VM_MTE_ALLOWED; return 0; ----------------8<---------------------------------- The fix-up for hugetlbfs is something like: ----------------8<---------------------------------- diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h index 8b9b819196e5..988eff8269a6 100644 --- a/arch/arm64/include/asm/mman.h +++ b/arch/arm64/include/asm/mman.h @@ -6,6 +6,7 @@ #ifndef BUILD_VDSO #include +#include #include #include #include @@ -37,12 +38,12 @@ static inline unsigned long arch_calc_vm_flag_bits(struct file *file, unsigned long flags) { /* - * Only allow MTE on anonymous mappings as these are guaranteed to be - * backed by tags-capable memory. The vm_flags may be overridden by a - * filesystem supporting MTE (RAM-based). + * Only allow MTE on anonymous, shmem and hugetlb mappings as these + * are guaranteed to be backed by tags-capable memory. */ if (system_supports_mte() && - ((flags & (MAP_ANONYMOUS | MAP_HUGETLB)) || shmem_file(file))) + ((flags & (MAP_ANONYMOUS | MAP_HUGETLB)) || shmem_file(file) || + (file && is_file_hugepages(file)))) return VM_MTE_ALLOWED; return 0; diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index f26b3b53d7de..5cf327337e22 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -110,7 +110,7 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma) * way when do_mmap unwinds (may be important on powerpc * and ia64). */ - vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND | VM_MTE_ALLOWED); + vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND); vma->vm_ops = &hugetlb_vm_ops; ret = seal_check_write(info->seals, vma); ----------------8<---------------------------------- We still have VM_DATA_DEFAULT_FLAGS but I think this is fine, the flag is set by the arch code. This is only to allow mprotect(PROT_MTE) on brk ranges if any user app wants to do that. I did not specifically require that only the arch code sets VM_MTE_ALLOWED but I'd expect it to be the case unless we get some obscure arm-specific driver that wants to allow MTE on mmap for on-chip memory (very unlikely though). That 'if' block needs to be split into multiple ones, it becomes harder to read. shmem_file() does check for !file but is_file_hugepages() doesn't, might as well put them under the same 'if' block. And thinking about it, current arm64 code seems broken as it allows mmap(MAP_ANONYMOUS | MAP_HUGETLB) but it doesn't actually work properly prior to Yang's patch in -next. -- Catalin