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 BCB86C282DE for ; Mon, 10 Mar 2025 16:31:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1B806280006; Mon, 10 Mar 2025 12:31:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F0FB280004; Mon, 10 Mar 2025 12:31:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E882E280006; Mon, 10 Mar 2025 12:31:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C3328280004 for ; Mon, 10 Mar 2025 12:31:44 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 568F8A9B5A for ; Mon, 10 Mar 2025 16:31:45 +0000 (UTC) X-FDA: 83206182570.04.5E88504 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf13.hostedemail.com (Postfix) with ESMTP id AD0502002A for ; Mon, 10 Mar 2025 16:31:42 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=none; spf=pass (imf13.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=1741624303; 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=wzcoIf96NUh1sWa/2HcfQvbgoDM1kvTg6m9Lj1x7EgA=; b=f7kTCk9F3gIOgQgscfI2AJ8vHJgJeEOg0qwNfTwobWBRkA1A/8uDCKDsSVa8ZOBwI1frVq hTxF2FWcAlaxS1GH78sGL5N0NydHh2sKV0JKZqha7Ho7vOXmaaBu9tPe3sESirnSSne1hQ 5SWRn0DjQb3tlpEAtVxETDk3IntA7AQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=none; spf=pass (imf13.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=1741624303; a=rsa-sha256; cv=none; b=G0Zt+rSBS4Q/2HN7bzPRdpKDywIZqA9wZZoks7o8wdTABhpkfjrMoPfOcwAns9gFTRXYg4 sZgosGYDZscOUvZO3JkwI2JBapKyPD7Qt+YmTwC303xIJAI/l8nSjt+5m8iNGmyhoR2tKu WGr07pHpaSmRqOEIvUlTX4zm0UmLywY= 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 C1D031692; Mon, 10 Mar 2025 09:31:52 -0700 (PDT) Received: from [10.57.83.165] (unknown [10.57.83.165]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 34D4A3F5A1; Mon, 10 Mar 2025 09:31:40 -0700 (PDT) Message-ID: <661898ee-c1f9-4f51-a73d-b70fe2eccd84@arm.com> Date: Mon, 10 Mar 2025 16:31:38 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] mm: use ptep_get() instead of directly dereferencing pte_t* Content-Language: en-GB To: Qi Zheng , Andrew Morton , Lorenzo Stoakes , Anshuman Khandual Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20250310140418.1737409-1-ryan.roberts@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: AD0502002A X-Stat-Signature: 7sk1tnasdn9jirokn7ibdiohkj3waajy X-Rspamd-Server: rspam09 X-HE-Tag: 1741624302-228528 X-HE-Meta: U2FsdGVkX19651s04FWWiSSJ8kf8xhVhsyc8Lcmwu/ACbRt464orTq/f0a8/zii/pE1iWnA5o9mfXnIuMTsAcbfWO941b5ScU6P6LqGD+j416iuY1W3t+8sSqkDmsLSxuFy4OOUnf/9pVYEHTAgDKR8Zq9eVmFxK0e5evLJ3F/HDDrYLxAuBUL6SJ26uwQdU7VcWwDe5XN8MszpJurhXu3kLueqzIZI28Z7np7z08e3VLXquM3aw9Ug0uxg4DBzYQkGhA6714Zz6FNC+GmpZTnAPFriHgvFM1pQClbx3muMmGfPBhrPICaaar/uKGmPq7aH7A5NFv+uZBXF4uWYPi/A2pUHe+MN2ygGEpPZb9rUI/i8u5NWMA5zMR3v0S50RQ80fEzYca2fdIJlgNoUmJYpDykTQeEt8nRzVJNTPWVWmzu9QQknm3jo7bfAq9W8KT9yc5QrX2XKrJweCcuL5b0zwPtFO2zCl4ZL4Ylhy5TON5PkQFLrFMIq88IqtonPiKQ7NqgYrP2f25HyLP2RHmoKCmNqd6Cy2/xq3blhD6KlIq7yP2BYRediUwSRrG6stowpSncGSG/X2Iau5GjtD9Q9MLDp9cYIeb6RA0QTHcE07fF34/pdq+MvsbBvKRJAayDf/dl8E6Rhl3P5emCW2YW0AiiUEHMgQgbF0aJXOGc2k9ao6Hbja37tdeA25pQdgF+K07Zg6pnI8GPa0+vp2kN1QRpuTkXJ3FOvifon4l6lCkv+FEJEBhLSNCnnAdKoO70nNxE7BzB8xto57jknvxEOyd1Kv7Y+FovuNt45DmiH6EUEsWpBVWNgMd4DYck9bcNDk7hAr4SCUt0P4FW7nWCDOkjY13/r6G7BXsKYubYKpcpPzAt8Lvs4yOlnWyF91GD1S8el0b6orKto8GCFmhAZ2J8Hld3gs7A5YOePdVVTFfgcti4jmaNsMnYP8D1Qjt+mAoGB4ryFdXf7ZaNA RYEwbmU8 ZPj63HFPZVcgTEeiLqew60elGioP8z1rO+yp8t8SYq9uWsK4tX2DhjTaV5B5a9rM6+hU9dLQxdpl/fDP2FXEOmI4jymkKGKnkA/HIqypjAEid+VP3UTDO1+Ckjgyp/mM/0itNDIKcNAxdkOMa3kNifBgISF6/dDX0HqhOD6VrbDnAUUm0MnUYz1r9L6/y+ECRV3SEsGoMm4/iVwXC8pfsoQzGYAyssihd9OLD1j1bY+gcvV0CNSE2H9DXdU7I4aM5VOc4YGzbvdOlrCXWESL/Sq54Pg6R2FMxQ9bHY+TFm4xsemc= 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 10/03/2025 15:51, Qi Zheng wrote: > Hi Ryan, > > On 3/10/25 10:04 PM, Ryan Roberts wrote: >> It is best practice for all pte accesses to go via the arch helpers, to >> ensure non-torn values and to allow the arch to intervene where needed >> (contpte for arm64 for example). While in this case it was probably safe >> to directly dereference, let's tidy it up for consistency. >> >> Signed-off-by: Ryan Roberts >> --- >>   mm/migrate.c | 2 +- >>   1 file changed, 1 insertion(+), 1 deletion(-) > > This looks good to me. So > > Reviewed-by: Qi Zheng Thanks! > > BTW, there are many other places in the kernel that directly > dereference pmd_t* and pud_t*, etc. It's all a little bit murky. For now, from arm64's perspective at least, there is only a hard requirement for ptes to be accessed through the arch helper. This is because arm64's contpte layer may apply a transform when reading the pte. In general there are also potential issues with tearing, if you don't at least read with READ_ONCE(). But often to consumer of the value is tolerant to tearing (e.g. pmd_none(), etc). Also, in practice on arm64 the compiler will emit instructions that ensure single-copy-atomicity for direct dereferences, so it all works out. That said, Anshuman (cc'ed) has been looking at supporting FEAT_D128 (128 bit page table descriptors) on arm64. The compiler does not emit single-copy-atomic loads for direct dereferences of 128 bit data, so he has been working on converting the other levels to use the accessors for that reason. But that has some potentially problematic interactions with level folding that need to be solved. Some arches rely on the compiler optimizing away the direct dereferences when folded. But it can't do that for a READ_ONCE(). I believe Anshuman is aiming to post a series to do this at some point in the future. Thanks, Ryan > > For example: > > root@debian:~# grep "*vmf->pmd" . -rwn > ./mm/memory.c:5113:    if (pmd_none(*vmf->pmd) && !vmf->prealloc_pte) { > ./mm/memory.c:5207:    if (unlikely(!pmd_none(*vmf->pmd))) > ./mm/memory.c:5339:    if (pmd_none(*vmf->pmd)) { > ./mm/memory.c:5490:    if (pmd_none(*vmf->pmd)) { > ./mm/memory.c:5996:    if (unlikely(pmd_none(*vmf->pmd))) { > ./mm/filemap.c:3612:    if (pmd_trans_huge(*vmf->pmd)) { > ./mm/filemap.c:3618:    if (pmd_none(*vmf->pmd) && > folio_test_pmd_mappable(folio)) { > ./mm/filemap.c:3628:    if (pmd_none(*vmf->pmd) && vmf->prealloc_pte) > ./mm/huge_memory.c:1237:    if (unlikely(!pmd_none(*vmf->pmd))) { > ./mm/huge_memory.c:1352:        if (pmd_none(*vmf->pmd)) { > ./mm/huge_memory.c:1496:    if (pmd_none(*vmf->pmd)) { > ./mm/huge_memory.c:1882:    if (unlikely(!pmd_same(*vmf->pmd, vmf->orig_pmd))) > ./mm/huge_memory.c:1947:    if (unlikely(!pmd_same(*vmf->pmd, orig_pmd))) { > ./mm/huge_memory.c:1965:        if (unlikely(!pmd_same(*vmf->pmd, orig_pmd))) { > ./fs/dax.c:1935:    if (pmd_trans_huge(*vmf->pmd) || pmd_devmap(*vmf->pmd)) { > ./fs/dax.c:2058:    if (!pmd_none(*vmf->pmd) && !pmd_trans_huge(*vmf->pmd) && > ./fs/dax.c:2059:            !pmd_devmap(*vmf->pmd)) { > > Would it be best to clean them up as well? > > Thanks, > Qi > >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 22e270f727ed..33a22c2d6b20 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -202,7 +202,7 @@ static bool try_to_map_unused_to_zeropage(struct >> page_vma_mapped_walk *pvmw, >>           return false; >>       VM_BUG_ON_PAGE(!PageAnon(page), page); >>       VM_BUG_ON_PAGE(!PageLocked(page), page); >> -    VM_BUG_ON_PAGE(pte_present(*pvmw->pte), page); >> +    VM_BUG_ON_PAGE(pte_present(ptep_get(pvmw->pte)), page); >> >>       if (folio_test_mlocked(folio) || (pvmw->vma->vm_flags & VM_LOCKED) || >>           mm_forbids_zeropage(pvmw->vma->vm_mm)) >> -- >> 2.43.0 >> >