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 262F9C3600C for ; Fri, 4 Apr 2025 03:03:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 416A1280008; Thu, 3 Apr 2025 23:03:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 39F25280005; Thu, 3 Apr 2025 23:03:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 23E0C280008; Thu, 3 Apr 2025 23:03:15 -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 036C0280005 for ; Thu, 3 Apr 2025 23:03:14 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id BD49DADFF0 for ; Fri, 4 Apr 2025 03:03:15 +0000 (UTC) X-FDA: 83294865150.03.DF0A04D Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf21.hostedemail.com (Postfix) with ESMTP id 9C62F1C0009 for ; Fri, 4 Apr 2025 03:03:13 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1743735794; 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=0He3sOFjcLng5OF7+ZXzck5D2d+Lox1ESeoAyDCInqo=; b=zC/Y5rddW8Q8DhUlCtdyzEZk/CBoGhNd6qNNik9ktKGl13OS8XOC95rdM3EwyXdb83dMXi EWsjWrG6yu7I/s+EcW35w9hqMrMfEJD8I/Zgofs9LVLj8tDcCnChcWUce9IHYY8UNW/s/j vdI86agt/f+K4HO4T+uEsJI0rCCeSKg= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1743735794; a=rsa-sha256; cv=none; b=njfCLP/Zzh6pdcaoo8UUvSX0mioeiP2cB9N8Xrm29YKnbOK2zwP8Lg9+9skG+TmKShfHl0 U8CFlOv4b4AWVPB6MdfDeQrWNYQffoAV4Y7KALv0sgNMzyWlB2jJ7YBt8LdfPS0d0JZyp5 4RrGUJ1MFqh4kOHuwLXBQzKR3TecIFg= 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 0E4741516; Thu, 3 Apr 2025 20:03:15 -0700 (PDT) Received: from [10.162.40.17] (a077893.blr.arm.com [10.162.40.17]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 84AB23F694; Thu, 3 Apr 2025 20:03:07 -0700 (PDT) Message-ID: <1d7c0c14-6758-4842-965c-2978c4a7f345@arm.com> Date: Fri, 4 Apr 2025 08:33:04 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 01/11] arm64: hugetlb: Cleanup huge_pte size discovery mechanisms To: Ryan Roberts , Catalin Marinas , Will Deacon , Pasha Tatashin , Andrew Morton , Uladzislau Rezki , Christoph Hellwig , David Hildenbrand , "Matthew Wilcox (Oracle)" , Mark Rutland , Alexandre Ghiti , Kevin Brodsky Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20250304150444.3788920-1-ryan.roberts@arm.com> <20250304150444.3788920-2-ryan.roberts@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <20250304150444.3788920-2-ryan.roberts@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 9C62F1C0009 X-Rspamd-Server: rspam05 X-Rspam-User: X-Stat-Signature: h4nrmubj9o9kfpy9jeu6mcp3tx6n8p89 X-HE-Tag: 1743735793-831529 X-HE-Meta: U2FsdGVkX18QhSJW8SpXTKyjHSvlDAQxkgdxvr+skNduhE/WoPSnBMINeEhQ3DBItmWI2MexkRSTqZNA4koKeh5WPcpgQuXG26jGp2XQNYBWART+6Bi2G1VwZ/0IzICH3YBMv4usv0rBuQNpmxoz/Or72HSMpcCYzIItLmUWr3rpftpo0yM0heU7O9Om3M9JyXPAw753juPSohSe+dXxMJwCH7r06sM2uvB3BvhsXelETrda61jUcp3D67q4/Z8eQRLhPrMrnbR7Ftfag+or3DX+bdtoBIlpsfozlySUOrna9G/KWP39RnJK2P2WWkv/n8AkRD+Insg1BwXr9uSOK0mQZGT085lhHgwP5X1OU8/L47V6fpQ3tEmXIsJjRRyCRYIFbUXm7Rk4aaVlwXzBFj24jd3HHAQBm6ybjCIyP0ZtF99RmIeGQmQlGj/e7pi+lMIdV9TUZi5EsXvilES8gCSriH82Yn7cRuo5gdcHTBGelhTTzxIYtdAF9oYChpuqWD59GR8Hfyk+VgxZPHXHIVdkShzREfacg8q7a6Bpb9zOSd26z0cgztHZmUD78YxXAZ+kShQhy5NBldKC5McZT4LLjsQIwXBrZDRFRpiHeAJm8GP+XcsWbhr0Qradj01VQ4AtrsM8PziQ5i5Obm3/L/46be+zjQkNIF6B10Gvf0WlRglDbybFL2DcPT34mEOhrGUPbI/CLOPtaLTomAE+JjIfFrI1NsG7kpXRXLXvEz7JlDnTaEmo2ZjraRfdyHY4wCIFXzY/8VLMBjBKxwqEfRjHaJ90nlDrNyI2Ikpl5CkcPH9/pAghIwhyo8LTwY65RAESKPpXlcOl0469WjtgGZopcs9E/mZKdwejV55vlQ1Dyo1b5prkZm5onN0yyC+6YM8NlkxVX9wAdQi6r98Kmd7vsYPpPbDrnWCh1DrsyulUk+5tilY1EFSUhaeXjM+gaENqZBZ1PX+CS/MFiz1 S/aMpGQ5 BSkxfdCkM+fN/ox/kkHTWoGP1KJCR3+IoPeioMdsqJh6FF+2gp2o79x3hVinb2QYvNPjhn7tvzAtbxVViN7G8MzMOH93CtzSvmShjTv+p4kN+8MVXHjVLt83zymFrO/hmQrWTGwHIhj3Hh8tJqUImWN6wNn3wVe6Ysw6O+Up1dP0c4a7brI6wWFq1yRCTaUVQjb246V3Nks9ki+XyIK5eoBFcvLSdi5LyO8O7L1WAkfD5QXvImSF+3YhPyQ== 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 3/4/25 20:34, Ryan Roberts wrote: > Not all huge_pte helper APIs explicitly provide the size of the > huge_pte. So the helpers have to depend on various methods to determine > the size of the huge_pte. Some of these methods are dubious. > > Let's clean up the code to use preferred methods and retire the dubious > ones. The options in order of preference: > > - If size is provided as parameter, use it together with > num_contig_ptes(). This is explicit and works for both present and > non-present ptes. > > - If vma is provided as a parameter, retrieve size via > huge_page_size(hstate_vma(vma)) and use it together with > num_contig_ptes(). This is explicit and works for both present and > non-present ptes. > > - If the pte is present and contiguous, use find_num_contig() to walk > the pgtable to find the level and infer the number of ptes from > level. Only works for *present* ptes. > > - If the pte is present and not contiguous and you can infer from this > that only 1 pte needs to be operated on. This is ok if you don't care > about the absolute size, and just want to know the number of ptes. > > - NEVER rely on resolving the PFN of a present pte to a folio and > getting the folio's size. This is fragile at best, because there is > nothing to stop the core-mm from allocating a folio twice as big as > the huge_pte then mapping it across 2 consecutive huge_ptes. Or just > partially mapping it. > > Where we require that the pte is present, add warnings if not-present. > > Signed-off-by: Ryan Roberts > --- > arch/arm64/mm/hugetlbpage.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c > index b3a7fafe8892..6a2af9fb2566 100644 > --- a/arch/arm64/mm/hugetlbpage.c > +++ b/arch/arm64/mm/hugetlbpage.c > @@ -129,7 +129,7 @@ pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep) > if (!pte_present(orig_pte) || !pte_cont(orig_pte)) > return orig_pte; > > - ncontig = num_contig_ptes(page_size(pte_page(orig_pte)), &pgsize); > + ncontig = find_num_contig(mm, addr, ptep, &pgsize); > for (i = 0; i < ncontig; i++, ptep++) { > pte_t pte = __ptep_get(ptep); > > @@ -438,16 +438,19 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma, > pgprot_t hugeprot; > pte_t orig_pte; > > + VM_WARN_ON(!pte_present(pte)); > + > if (!pte_cont(pte)) > return __ptep_set_access_flags(vma, addr, ptep, pte, dirty); > > - ncontig = find_num_contig(mm, addr, ptep, &pgsize); > + ncontig = num_contig_ptes(huge_page_size(hstate_vma(vma)), &pgsize); > dpfn = pgsize >> PAGE_SHIFT; > > if (!__cont_access_flags_changed(ptep, pte, ncontig)) > return 0; > > orig_pte = get_clear_contig_flush(mm, addr, ptep, pgsize, ncontig); > + VM_WARN_ON(!pte_present(orig_pte)); > > /* Make sure we don't lose the dirty or young state */ > if (pte_dirty(orig_pte)) > @@ -472,7 +475,10 @@ void huge_ptep_set_wrprotect(struct mm_struct *mm, > size_t pgsize; > pte_t pte; > > - if (!pte_cont(__ptep_get(ptep))) { > + pte = __ptep_get(ptep); > + VM_WARN_ON(!pte_present(pte)); > + > + if (!pte_cont(pte)) { > __ptep_set_wrprotect(mm, addr, ptep); > return; > } > @@ -496,11 +502,15 @@ pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, > struct mm_struct *mm = vma->vm_mm; > size_t pgsize; > int ncontig; > + pte_t pte; > + > + pte = __ptep_get(ptep); > + VM_WARN_ON(!pte_present(pte)); > > - if (!pte_cont(__ptep_get(ptep))) > + if (!pte_cont(pte)) > return ptep_clear_flush(vma, addr, ptep); > > - ncontig = find_num_contig(mm, addr, ptep, &pgsize); > + ncontig = num_contig_ptes(huge_page_size(hstate_vma(vma)), &pgsize); > return get_clear_contig_flush(mm, addr, ptep, pgsize, ncontig); > } > Should a comment be added before all the VM_WARN_ON() explaining the rationale about why the page table entries need to be present, before checking for their contiguous attribute before subsequently calling into find_num_contig() ? Regardless, LGTM. Reviewed-by: Anshuman Khandual