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 B19ACCD4857 for ; Wed, 4 Sep 2024 15:50:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3B4406B0123; Wed, 4 Sep 2024 11:50:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 363086B032F; Wed, 4 Sep 2024 11:50:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 205316B0165; Wed, 4 Sep 2024 11:50:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id EA2D58D025C for ; Wed, 4 Sep 2024 11:50:18 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 96ACC8170F for ; Wed, 4 Sep 2024 15:50:18 +0000 (UTC) X-FDA: 82527492516.12.BC0ED39 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf01.hostedemail.com (Postfix) with ESMTP id 8C66F40008 for ; Wed, 4 Sep 2024 15:50:16 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725464968; a=rsa-sha256; cv=none; b=BTv4G/crmrKMjDdu8WuNoRzHlFPmfuD/cm/EpxVEzI3W+D3XnF56OFNyOd9iSQMkAnq5Er xHlH6tOt+mWfoSyxsoU7F2DMaLlC37od/F3P4fIxRJjUfA57ECo/A4mmWEjHSb8na6Yvfs 2vb9nQ8d7GBJzHuCcAchwskOdu+AEuM= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725464968; 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=S6YG1PNTNfujZOVyxDmKMKD+K4ZVreqAYpx8R6z04lY=; b=g9S1ueevN/8QamyvweHM6peZHxl8g+2jpdpVGrc8k2/2xY5ZO5hSgcImBufKCtWwlEDbc/ xkcGoL57gQvgOY0XBCNNk4JgHZ+HFRDkhWG5QOvuwr2ZSRFttmefQhD7obLV4wuL3mR/HY zErb6So+yzq2oQhfAq/U1rHYOudUdag= 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 9D721FEC; Wed, 4 Sep 2024 08:50:41 -0700 (PDT) Received: from [10.57.87.65] (unknown [10.57.87.65]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 918463F73B; Wed, 4 Sep 2024 08:50:13 -0700 (PDT) Message-ID: <629ee1a6-c606-4a8d-bfd6-a2be31feddcf@arm.com> Date: Wed, 4 Sep 2024 16:50:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RFC] mm: arm64: advance pte for contpte_ptep_set_access_flags Content-Language: en-GB To: Will Deacon , Barry Song <21cnbao@gmail.com> Cc: David Hildenbrand , akpm@linux-foundation.org, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, Barry Song , Ard Biesheuvel , John Hubbard , Mark Rutland , Catalin Marinas References: <20240831083537.62111-1-21cnbao@gmail.com> <20240904151304.GB13919@willie-the-truck> From: Ryan Roberts In-Reply-To: <20240904151304.GB13919@willie-the-truck> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Stat-Signature: r9983r1jfz4sbe3qtijjyb3foogs6osn X-Rspamd-Queue-Id: 8C66F40008 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1725465016-415596 X-HE-Meta: U2FsdGVkX1/dXkBXebtwGOPfLiuCKxZrVU2AObhd0pmQOmwp0uqE7CesXy9P+F+SbKVzw79xD1ofaxJUQSSnQD3VWWPGlOFB1A8BVGFO3ZQuWU+M9UcVqwi8Lw7rS0jxtOdoWwGolZWwy23oXdAIsiEh5OybL+eMiHwuRmlsF+2c0ipJVdATft+iNpb/E04mjSe1Z2pVAmf3U07Tg1vbFq6XQkiE4etDKOk9mNYJ6fCinL/vQyCVlGv7FK7SCBe1xCG+ER95zn6O9a6Mm9spagHEvGhXnA+uPs849PtMznKMFRkogJYsiIA41J+5/qkOo1PWUxWapBVqoho+BvuY+GSTp4bN/NDtgUMncGWV5FsOXXob59Ra9TxAONXMQhO6pVrTwjHY44jKFx9gpm3cepSEklVDPm+1eDeaVKbL/hePCZTBeFY4zTlbtmAtUNGQDWsS54f6EEUi0K3KiaR44+SAODNlzKxwCWpcRQrYphcS0k348eQX2svvuZuqJ8UfUHcd8SFdGp71tDj8ib3wOqjerD/FulxYR8IPcHdy5pZlhBbpyqDlgNIX+i87C6bNi9894XzhOYKYEcK29U2U6hNG6VivhauXmmW9yaqPF+fwK5Wlen8hL9AcFaLW6xM7c2ZelhXr2JUWo/+BRO/8dbD1VpNb15Tv6xKvfP1QVa5B4BRH0JgvdxpHI5M9SfA7X5PTr+IOsgnOUtFdlN3qhxxk4QHNHO5bRHma014DKf+X/PCxjGPNWAQnAtrmk+V4GXssCwBQ7WdtDU6N4KZWjO6Kahd8jmzV3c5+nhumXPT7j1eyo5vFFbmIKTqxhN63X5rhzAGKoN72mya4FjVPjf9URJKuqQCjSb1NmCbM+YlCrpxCXpWbyQ4rFTirDYM/U6egtml1e8rj6D9H+8fjMVbXiK6otaHlOnX4hzJbF0Gjs7EUQLAvs+dl6e65tvk5k4t5W4n10K5liNg2Daf UnAsPsLu zI5YdddS8DR2XhSrbWSgEOdkd+Fl99v6z94we+IOOn1r0720gPl788Qmm0yJpFv5uHe5QK7O4HCn8xNWIw8LZJdCZbc/eKqVotp7ktTgvVh8mFhAYYa+27ETdDXzCePSFTWYsIhVATBV/veD8tJCzej3dfGkk00H8KZVoWv73LMrDjUcFkQKVRKsEzgCyxD4ylGeUdWZt94eTolkhnfYLThNaD9l9A33oO9L99vXhaP3uXgAP647Lc5LI4xFYBUH9gMUmWGXLtFN/3rV2hrnOQjJIXYniPjJhXltTy3G7IYXoCY84o71wx2z2Agz5eHh0Gq88tWaug/wrMB4clKypkelGyBP/RMtUHV5SAPvUnh9yGn/gdFzJBxdiYg== 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 04/09/2024 16:13, Will Deacon wrote: > (Adding Ryan, since you're asking him a question!) Thanks, Will! Afraid I don't do a good job of monitoring the list; I'm guessing there are automated ways to filter for mentions of my name so I catch this sort of thing in future? > > On Sat, Aug 31, 2024 at 10:06:40PM +1200, Barry Song wrote: >> On Sat, Aug 31, 2024 at 9:54 PM David Hildenbrand wrote: >>> >>> On 31.08.24 10:35, Barry Song wrote: >>>> From: Barry Song >>>> >>>> Hi Ryan, David, >>>> it seems contpte_ptep_set_access_flags() has never advanced >>>> pte pfn, and it is setting all entries' pfn to the first >>>> subpage. But I feel quite strange we never have a bug reported. >>>> Am I missing something? >>>> >>>> Fixes: 4602e5757bcc ("arm64/mm: wire up PTE_CONT for user mappings") >>>> Cc: Ard Biesheuvel >>>> Cc: John Hubbard >>>> Cc: Mark Rutland >>>> Cc: Catalin Marinas >>>> Cc: David Hildenbrand >>>> Cc: Will Deacon >>>> Signed-off-by: Barry Song >>>> --- >>>> arch/arm64/mm/contpte.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c >>>> index a3edced29ac1..10dcd2641184 100644 >>>> --- a/arch/arm64/mm/contpte.c >>>> +++ b/arch/arm64/mm/contpte.c >>>> @@ -421,8 +421,10 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma, >>>> ptep = contpte_align_down(ptep); >>>> start_addr = addr = ALIGN_DOWN(addr, CONT_PTE_SIZE); >>>> >>>> - for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE) >>>> + for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE) { >>>> __ptep_set_access_flags(vma, addr, ptep, entry, 0); >>>> + entry = pte_advance_pfn(entry, 1); >>>> + } >>>> >>>> if (dirty) >>>> __flush_tlb_range(vma, start_addr, addr, >>> >>> Taking a closer look at __ptep_set_access_flags(), there is: >>> >>> /* only preserve the access flags and write permission * >>> pte_val(entry) &= PTE_RDONLY | PTE_AF | PTE_WRITE | PTE_DIRTY; >>> >>> So it looks like it doesn't need the PFN? Correct, I don't believe there is a bug here; __ptep_set_access_flags() only consumes the access flags from entry. >> >> right. >> >>> >>> >>> OTOH, there is the initial: >>> >>> >>> if (pte_same(pte, entry)) >>> return 0; >>> >>> check that might accelerate things. There is an equivalent check in contpte_ptep_set_access_flags() which is checking for the whole contpte block and returning early if so. So I don't think there is a problem here either. >>> >>> So unless I am missing something, this works as expected? (and if the >>> pte_same() would frequently be taken with your change would be worthwile >>> to optimize) >> >> >> Right. From page 1 to page (nr_pages - 1), we consistently get FALSE >> for pte_same(). >> This seems quite strange. I think we might need to "fix" it, at least >> for the sake of code >> semantics. on the other hand, if pte_same() is not important, it >> should be dropped. >> >> Hi Ryan, >> what is your take on this? The code is correct and working as intended, AFAICT. But I accept that this is not exactly obvious. I'd be happy to Rb your proposed change if you feel it clarifies things. Thanks, Ryan >> >>> >>> -- >>> Cheers, >>> >>> David / dhildenb >>> >> >> Thanks >> Barry