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 8D041CD4848 for ; Thu, 5 Sep 2024 03:27:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA4A78D0005; Wed, 4 Sep 2024 23:27:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D53C48D0001; Wed, 4 Sep 2024 23:27:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE89A8D0005; Wed, 4 Sep 2024 23:27:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 9A8758D0001 for ; Wed, 4 Sep 2024 23:27:26 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 26D7012187C for ; Thu, 5 Sep 2024 03:27:26 +0000 (UTC) X-FDA: 82529249292.30.B41F68E Received: from mail-vs1-f41.google.com (mail-vs1-f41.google.com [209.85.217.41]) by imf20.hostedemail.com (Postfix) with ESMTP id 630291C000C for ; Thu, 5 Sep 2024 03:27:24 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=HeVsGlye; spf=pass (imf20.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725506795; a=rsa-sha256; cv=none; b=KD6lat0vXbR2v8BNRR0E8S6HHf8oAO5R3HCi/Ok+RSd4lYvon/ta97nzaFFsrAt823vE4I Qk9WCxK6WbV1SKoiOpGX1enAL+q7a3Rh/WbET0Z/DSYJHTCWtqWJEsh4ZyxAO0K1Zl7zey A/abP2l8JfOcGy++o+s090QJ7vrf5is= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=HeVsGlye; spf=pass (imf20.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725506795; 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:dkim-signature; bh=dE+/1wHvtIIvED9iIAdwtghP0G4eXTxZSd/+k8fLa1M=; b=et8t2eqrKPHDFE9vBB02BGy1TyHeCEM14QaGd5ObRegYSN8tTCp+ASdIsnIBTn3yLnAapl rS+13Q5NerV+xEhuNVy6PHdp+97UzWNYoPsgtNQ//6M6P8XATtll2V++lUYsY/qjxXDVRj j5y3lMxvLYZwHdDEFN99lpQJ6b/CyL0= Received: by mail-vs1-f41.google.com with SMTP id ada2fe7eead31-498c3d1d788so81577137.3 for ; Wed, 04 Sep 2024 20:27:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725506843; x=1726111643; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=dE+/1wHvtIIvED9iIAdwtghP0G4eXTxZSd/+k8fLa1M=; b=HeVsGlye0Dmmw+8UEnGDqRD10zm+XWASsY8Lp1rURAZK2TwSlwJNFbGde384sEE1sJ ngSnfBQNwTpM0Rn5h6VxMOPd/TT00ClHhJXdKaAvRbHvZl0EDA0GTeV5tEZDokV0p1H8 E5X7N2jXkLjetxbtJZ9zTyh3eCZnqazF3nGAQdcSELh4SVFmWuex+iaPD16p6yLhhrC/ U/8TUPVcf+gGOjlVsj0zoB+wt77IvROe6tlWHpmRoYAhfbIcbfo943yXMWPDX+m/ItZ2 V07lOcK3GFortgGE+SPOjua8+64ricI9FGJvIfJGoaPGNde8r0PwDNYNVSLnP9b+Dp50 Rn+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725506843; x=1726111643; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=dE+/1wHvtIIvED9iIAdwtghP0G4eXTxZSd/+k8fLa1M=; b=A2UhAiJ9/uWkCbiJ3Ia0gH2KSj2muXiLgulG9kuMnFgFrBoerEK3bPXOFsArPcJ4+G YGfyMQW5XDsW3VyjLGePZdtlNilp2HvaZ44uCGutT9mlgRNVZycuUZdDQQ89udPZd0TZ 6AXDDglWkskqMN/hBsiOQmIeL/w5tjB9Mm84bljBwNJxWy7kbuaPKgKmVi30RM9aHtZ8 cIcarmrgNDnSsvL4pXlkV6xFjC11TaV59p7Hi12Y0B2HSL8RotnPwWJuBECJawNd3EFR U4UcGDw+O898F3LpUhxTcaUsIQGl+qFOKAjUNWD6yc0l+RkixReXgrtdFPp6a2/yoeyM hB2g== X-Forwarded-Encrypted: i=1; AJvYcCV7Gs5uRYa8TJ1y+yW7T7btnKg51WfdvyGe9lrG+A6DOmqHZH3DtmJTzr6lSwQ9OQHrR0rbKMPyBw==@kvack.org X-Gm-Message-State: AOJu0YxQi6EqXADZLCWuo37aMqRzctJdtsI8wR2R5BwNjmKNDAVUOaUz IrsQXR75pBMeVoRsXGeYkXiz/yC2x1Mtl0uCS/YVLqT7wCr6G/qUHxnH1Vdj2KcJCxnHf+MRAvL IuyENlkTB5+Z5ST1QZhKF9djufec= X-Google-Smtp-Source: AGHT+IEbjQjYXxIlMmh/6a7S4anWEERLMIgRm44pitmLyOqO48acbxcE0zh2qIuebhqTIYWfyKnZ8nqSzxMf5F4BuZA= X-Received: by 2002:a05:6102:a54:b0:498:da59:4e78 with SMTP id ada2fe7eead31-49a77984c0cmr13593036137.12.1725506843257; Wed, 04 Sep 2024 20:27:23 -0700 (PDT) MIME-Version: 1.0 References: <20240831083537.62111-1-21cnbao@gmail.com> <20240904151304.GB13919@willie-the-truck> <629ee1a6-c606-4a8d-bfd6-a2be31feddcf@arm.com> In-Reply-To: <629ee1a6-c606-4a8d-bfd6-a2be31feddcf@arm.com> From: Barry Song <21cnbao@gmail.com> Date: Thu, 5 Sep 2024 15:27:12 +1200 Message-ID: Subject: Re: [PATCH RFC] mm: arm64: advance pte for contpte_ptep_set_access_flags To: Ryan Roberts Cc: Will Deacon , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 3u8p7yqwunprwysegj7ody6qbo7okssz X-Rspamd-Queue-Id: 630291C000C X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1725506844-191980 X-HE-Meta: U2FsdGVkX18QJrHcO/ImQuRbAIG1qh6ZJ9rU2a6caldyxeZzXDgpp+F3j8EtpwYDkHhgH791b2mJ6H9nHPLB+JbtLwcNuxoQ+HrMh6ZtrqPNEq2YRWQDBs//DJ1JBAm+Xih0u8k+VSSGXLrlkThBc+dqRBBXfM0yrzJKBKMCQ3JATtyySY+Gf4ZUcak594HQ1eWP7vKNFvEbNnChQzr0S++IEYuCCMRwJsPG/1WPTt3y4VyjiLhPM3MDG7meSZ14Yz0U8Acs57gKCLWD0qhOZ07y1mav5RCoX2Y0U0OAivemFmnbM399+HgAqaezaZD++mwc5eZqrO5oqP4cgwnXyjk0LP5hdA++S6m4VtFqxq6X5Y8o+7xiJXMKk7DHQOPzdfA23RohoD+lxxQYKVwuR2FLfyi/nw26ogdZv6D+cfU0yo5U41D0m37NbTrYEBpNEMzimL5LRfqOiVlczNyVauzmSYsjnhRwewfYNomLHFqPfENgHbRCgLFroeMPduvbMvrLH9zbrkmycmGSHpQZBDVwpQKWFpLmT8h9GZzwxTOMMiXOT6rKWbi1BDreEM/vpDqcAkjVB5Tke9TBKJIN0hRxi4qy7WcgG5lU6sCUusbIjPmEmdGLPUF7Yf2uj3PKlYTkwEwIggI2eR5W4gmQIV98yPloMHwnVb4iSxhTp81T9ANFD29QKWh0KmJPrFET74EsbETHttoaRxNxh6tq5JZ1zAY97rW/l1adtXvq1DJANj/fAJD5FodwlyZJV/XGHkJlQ0HIA5abTmBdEnUuhJ1WQ0tobIsc6FnHbioY+Hx7oOUHnboB4k4nKv9qey5QEBmsNlhHTmI4A7xfPnoeHfiRD+YAhATboKlrt8nRQWnubGLKwHL3omF9Ocp1qjfce6QIC1w309rrWJIWQOzx/MLggFwWzdyHtF0N/al22W8neOhAD81+bwqQb6k6zlBK2K00HoL+D3OPN+14XdO iNPaOKV3 nq+mT28NYN32AZsrj5XRgkM1rvS5Toxuw0x3pqKFfP/xkTSRBlSQoGUVW0OByIutgVH804ZqHuI4+M+wS4w+PV5yq1HSWcAGI3Omq0N5YEr2gmHSy0A9un+MJwvry9r9Qwfkr1ogQQuJEcbR7CnbLRGMa+JU8bQwlPPGx6Lyp+C3crU17eVgkYW1k8EXGN3Tw4VwVpRK2XyFtMVucxNjAodE8nFGkiGbTZkY2eOxXiB5fsTRaFXFdCR4aMsh5s8JCA0IIMdTMWcHrc8rKMEd3mjRlybUnvi6jT01p4o6tcrAHRUMxkwMIPVloQ1t2bvpDIQrzDNOPnKnqrwA7iq7OUh5Rv7fVZh/OW/IRC9J4yKqREHHalqdv08kejqpmEtZMGalQPyrRiiAThlvsOYtne7ot2/OG5rRFvSWtyzz/sd8fIQCZOvk9ymJ8uTYGrmSwbYi6nffBkiFTZfuRPHopScGr8H4PHMb41eqcfDq1pwJjO3Yq2TMcsKednSs37JSAikwbS2ruK6F7JxDa264OXBbLkCTO/VYRVHablI2mRDjWdMSoDeDX9+lWnw== 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 Thu, Sep 5, 2024 at 3:50=E2=80=AFAM Ryan Roberts = wrote: > > 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 a= re > automated ways to filter for mentions of my name so I catch this sort of = thing > in future? It's not your fault. I just realized that, for some unknown reason, I forgo= t to CC you. > > > > > On Sat, Aug 31, 2024 at 10:06:40PM +1200, Barry Song wrote: > >> On Sat, Aug 31, 2024 at 9:54=E2=80=AFPM 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_are= a_struct *vma, > >>>> ptep =3D contpte_align_down(ptep); > >>>> start_addr =3D addr =3D ALIGN_DOWN(addr, CONT_PTE_SIZE= ); > >>>> > >>>> - for (i =3D 0; i < CONT_PTES; i++, ptep++, addr +=3D PA= GE_SIZE) > >>>> + for (i =3D 0; i < CONT_PTES; i++, ptep++, addr +=3D PA= GE_SIZE) { > >>>> __ptep_set_access_flags(vma, addr, ptep, entry= , 0); > >>>> + entry =3D 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) &=3D 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() o= nly > 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 worthw= ile > >>> 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 th= is is > not exactly obvious. I'd be happy to Rb your proposed change if you feel = it > clarifies things. If this is the case, I'd rather add some comments instead in v2? diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c index a3edced29ac1..55107d27d3f8 100644 --- a/arch/arm64/mm/contpte.c +++ b/arch/arm64/mm/contpte.c @@ -421,6 +421,12 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma, ptep =3D contpte_align_down(ptep); start_addr =3D addr =3D ALIGN_DOWN(addr, CONT_PTE_SIZE); + /* + * We are not advancing entry because __ptep_set_access_fla= gs() + * only consumes access flags from entry. And since we have checked + * for the whole contpte block and returned early, pte_same= () + * within __ptep_set_access_flags() is likely false. + */ for (i =3D 0; i < CONT_PTES; i++, ptep++, addr +=3D PAGE_SI= ZE) __ptep_set_access_flags(vma, addr, ptep, entry, 0); --=20 2.39.3 (Apple Git-146) Thanks Barry