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 31926C2BBCA for ; Wed, 26 Jun 2024 01:23:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A39536B0093; Tue, 25 Jun 2024 21:23:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E8E06B0096; Tue, 25 Jun 2024 21:23:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8894A6B0098; Tue, 25 Jun 2024 21:23:21 -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 6B7336B0093 for ; Tue, 25 Jun 2024 21:23:21 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 1323DA045F for ; Wed, 26 Jun 2024 01:23:21 +0000 (UTC) X-FDA: 82271291802.29.880AF70 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) by imf18.hostedemail.com (Postfix) with ESMTP id 257511C000B for ; Wed, 26 Jun 2024 01:23:18 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AjHlJvv4; spf=pass (imf18.hostedemail.com: domain of npiggin@gmail.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=npiggin@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=1719364990; 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=G4B0rJOt+TwGvnGU5npNqzbhof2wF8oGmpu7vqDya1w=; b=HCWkZrO1y1swfmeNTCCsXt+k4xnxaso9EW4ZXYIwY+mLfZl9l5yHoXXLUKhM+hxVrxuBEw Lxt3KpfytzyG7HfGSqpN2H4Xpo/tAwixhXl6a5VIcenb1HD8k2dLu4FR+33y//eOSVHzSM l+remyXft8Dc0Mpf3sRTCeqc3zFaU7A= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AjHlJvv4; spf=pass (imf18.hostedemail.com: domain of npiggin@gmail.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=npiggin@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719364990; a=rsa-sha256; cv=none; b=yR4GD2DR3QeV8nSfeF4jb6GYOqVeE+NFqJ5Rg3plRhqgpK73sCZQgOYx/cUcKa05n7/6s/ NhkYIRJm33OwPjWjLHA0V+Bdsraz+A9goXU3NzBCOZ8gNIG7UeWI9xmba9r0CS4PmxBFDY X1t2F0QNl4RWQEgT2W8BW0rCPC2JLq4= Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-1f9b523a15cso531395ad.0 for ; Tue, 25 Jun 2024 18:23:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719364998; x=1719969798; darn=kvack.org; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=G4B0rJOt+TwGvnGU5npNqzbhof2wF8oGmpu7vqDya1w=; b=AjHlJvv4Aet0WTnajMJCAzJaeti7PhkPC1S3QWziHBHUlJfUTVTYFigMA513qpgcta pvwcLMrEIPJ61ZrKTYlKb6u0GTl0XUIEZft3J4Rbh4iMGgUbnQLhA/Js0k2CJUSojf5P iO/wBxpRsVdnQin1g7OhDq3ILUa/XeJ7kYanz0yVJjgMtMtj/Ffm7D20rZ9aXpy5FPJN p5nZX2N8WKAtEhzo/OxM2wYY+aJPsTveHqw//6NyXeG+hq8QxUfEyT1+25Eju/p+sNUu ORueeM2aBRM/LW6xhSF6hHC6c05URNMSqJIWq6BVF4YANwQpMBsLiVI3uYV6Q+ZC6SP7 ldlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719364998; x=1719969798; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=G4B0rJOt+TwGvnGU5npNqzbhof2wF8oGmpu7vqDya1w=; b=HooyT0eJAFJvozfEMSokeVK/wcImKArY9qSQd9tT5H8hBRoMVovPlF7aaYx2qrVE5T F7+xMq6dZEt9+nBCVCn/tDbwHN+GVUejslBXvnUgZv5+LsuI5lBSkxRdIUhUyAGzDmaI DgpZJ88lU7Z+k8DkELoPlzqLTdCDNAzPHSQxEmoBfWWbUtrOYMk1/lVsn5m0FcGLDWdI sepzKLBQ4vvYmgsU0E7NOMTgJd1Fahv7WGsXAHxe+iygcoE+o1nPFQmenQ85OC2CDgL4 e0Is7di5Nvj72siOK07T91v+HI4+TPuiae3B2zp90d6fHKb1u++lMAfjORpW+PKUIabG VEIQ== X-Forwarded-Encrypted: i=1; AJvYcCUPwN49vtiV/Uqpx6+EAsx1UUYDfBVkrtu5FmCiOkm/xJ1ZK6KxkRDllBUjzFhKtySq6wrQLDH0UGETO+NfJneXcws= X-Gm-Message-State: AOJu0Yz9bes/b8xOY05o5EDe8VC9jhN78ZfatRvoHcGrO0cAO10V/o34 adWx8qPCc1/el2d0J925aKllNA6sc3m7utlqEbT3k3i7QNY/WyjA X-Google-Smtp-Source: AGHT+IEqVqDw1A2NGO/alG4xT0pZfiNKGUIkmcWKhLu+8DBykPZWCX+p598Z0xCi0cvGZzLtolmp2Q== X-Received: by 2002:a17:902:7441:b0:1f4:a04e:8713 with SMTP id d9443c01a7336-1fa5e6bde38mr60633605ad.28.1719364997855; Tue, 25 Jun 2024 18:23:17 -0700 (PDT) Received: from localhost (118-211-5-80.tpgi.com.au. [118.211.5.80]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f9eb3c5d29sm87734655ad.154.2024.06.25.18.23.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 25 Jun 2024 18:23:17 -0700 (PDT) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 26 Jun 2024 11:23:11 +1000 Message-Id: To: "LEROY Christophe" , "Andrew Morton" , "Jason Gunthorpe" , "Peter Xu" , "Oscar Salvador" , "Michael Ellerman" Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "linuxppc-dev@lists.ozlabs.org" Subject: Re: [PATCH v6 21/23] powerpc/64s: Use contiguous PMD/PUD instead of HUGEPD From: "Nicholas Piggin" X-Mailer: aerc 0.17.0 References: <23f3fe9e8fe37cb164a369850d4569dddf359fdf.1719240269.git.christophe.leroy@csgroup.eu> In-Reply-To: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 257511C000B X-Stat-Signature: h6coqh5axkqtasq3qcofoji9cxtgr9ti X-Rspam-User: X-HE-Tag: 1719364998-146177 X-HE-Meta: U2FsdGVkX182mqzyqqAxOn+FGfRWbHlmXbtOX49wtvYGZRBP1WWmWpBHnAt0f/WOVjpLIwTzrdPWH6dLw2rufzfNcDy+SQbGYF5uyF7Ap8LP2HtjnId+J6zTppUGV67pbL6VjtqyeGxL28zRWbQEJMh8bQx21QKyAccnkgTSJYNHcTqestuEJTjRvCWxCSWoRbYYZ5goYLwxBtKWYwNDNhG4stNe/2y3f1Uqq9GitJaRUITGs6VMctQlS4PrN7pzbyAXkmaCL+cv7teFt2VD3l3ckViQ6e4+ZrigjYr4ZtsPdL975DHFagX2jx/QtKBvHlN+8zLKqqH6gkUv+BT+AOTOpQkgOElaCkbVyCINHL9IgDGQpNNvuDRSfXMqZEonoUJh34zQ9/LBOHraCcok2OgYh+Gg/297+o6k10GxTvwu6NTBhHizRuANsUIDP9lNYLXgqMUoFl09+ED0R+ugujuMu5yzxwg/OZyakKeBuXcGkPl+GNRAML53bYcaFIv+xrrwJC+L09T+dIj0GDvasmJb+oTnv3BVuArQY4D8Q/lj/nWdvuseDui0tXJHpMCOhTX9WCSbJ43/0b3I5HTBa44Q9dZ5jPJcBuh3USvitdZu+st7ulXyjFErJCg8W96HtCOOYiD2QG4d6ABKXtq3zgkHjWHZeGmRj+xQrx/dmrIvGH1OemJTRz0wCunDdYiUV9y8Yy4Z0L8Nn+JiPIHYYGhV6isg0w1pt3HC2Q491VBNV3uvQyxBWRkpgK8q3m5ho7oJDr/kEzl5THfSFI5BNQZVYgz0b7zgkp7XrGrsn3iDZGPJYIv6T8cowlzILyD0dC5FBJum4ll6MdWq+3H791cXGqS3/jF1Qz+a9LD8YUrUFE1lSbaviKJ54N0F39zEnXfiPqQegegRDFMtJbnlqCSTnxYUKnHxQdGha6XNv8Xe+6bQCPmDcp2JZAZrCV1/298sn+Gsu4ggtmeZdVS IZK1Wsav c9ASKfW48seIbdB96k0h1C1N33eQSmiTzUJOzrZRuBmesbbo7lZogk5FmJtb2s/8rDYi3UzWKzFa4iwm83kpRvvEnGlATeugKGhGLoJ4QxoeN195DwHDYeUrrM/Sxqev9buiDlltJjfL5Akf26UPyEAGyOlx3q/MFfEcNblVtt8snLI3XUl77s49Wm77Oa8jnuPb8fxjOOEkfPT3zIiSPJtTmzM1YGZVz/sTaT4duc5oOmDp5U+mw3af2lMSrn2AmxRi37UfCdS3PP0G7JL19NJfAdHZ6HcOG+3VtO3iXTarZHtUupuk5nFpoW04Ia0mzu/EGUvCRDMW4z1Uqy+svnSVT2hWYUSNAlGauteKmO8FO6M/kex6oB56oYPcRbIoGz9KTPlkckG8tZOO9poDCfUy43SyD/SpZnj8sadLvV/HBWcVQll/IZOQYiCvop6KVVam5PQYWoh3Th8pZAry32rovwQ== 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 Tue Jun 25, 2024 at 3:20 PM AEST, LEROY Christophe wrote: > > > Le 25/06/2024 =C3=A0 06:49, Nicholas Piggin a =C3=A9crit=C2=A0: > > On Tue Jun 25, 2024 at 12:45 AM AEST, Christophe Leroy wrote: > >> On book3s/64, the only user of hugepd is hash in 4k mode. > >> > >> All other setups (hash-64, radix-4, radix-64) use leaf PMD/PUD. > >> > >> Rework hash-4k to use contiguous PMD and PUD instead. > >> > >> In that setup there are only two huge page sizes: 16M and 16G. > >> > >> 16M sits at PMD level and 16G at PUD level. > >> > >> pte_update doesn't know page size, lets use the same trick as > >> hpte_need_flush() to get page size from segment properties. That's > >> not the most efficient way but let's do that until callers of > >> pte_update() provide page size instead of just a huge flag. > >> > >> Signed-off-by: Christophe Leroy > >=20 > > [snip] > >=20 > >> +static inline unsigned long hash__pte_update(struct mm_struct *mm, > >> + unsigned long addr, > >> + pte_t *ptep, unsigned long clr, > >> + unsigned long set, > >> + int huge) > >> +{ > >> + unsigned long old; > >> + > >> + old =3D hash__pte_update_one(ptep, clr, set); > >> + > >> + if (IS_ENABLED(CONFIG_PPC_4K_PAGES) && huge) { > >> + unsigned int psize =3D get_slice_psize(mm, addr); > >> + int nb, i; > >> + > >> + if (psize =3D=3D MMU_PAGE_16M) > >> + nb =3D SZ_16M / PMD_SIZE; > >> + else if (psize =3D=3D MMU_PAGE_16G) > >> + nb =3D SZ_16G / PUD_SIZE; > >> + else > >> + nb =3D 1; > >> + > >> + WARN_ON_ONCE(nb =3D=3D 1); /* Should never happen */ > >> + > >> + for (i =3D 1; i < nb; i++) > >> + hash__pte_update_one(ptep + i, clr, set); > >> + } > >> /* huge pages use the old page table lock */ > >> if (!huge) > >> assert_pte_locked(mm, addr); > >> =20 > >> - old =3D be64_to_cpu(old_be); > >> if (old & H_PAGE_HASHPTE) > >> hpte_need_flush(mm, addr, ptep, old, huge); > >> =20 > >=20 > > We definitely need a bit more comment and changelog about the atomicity > > issues here. I think the plan should be all hash-side access just > > operates on PTE[0], which should avoid that whole race. There could be > > some cases that don't follow that. Adding some warnings to catch such > > things could be good too. > > That seems to be the case indeed, as we have the following in=20 > hash_page_mm(): > > #ifndef CONFIG_PPC_64K_PAGES > /* > * If we use 4K pages and our psize is not 4K, then we might > * be hitting a special driver mapping, and need to align the > * address before we fetch the PTE. > * > * It could also be a hugepage mapping, in which case this is > * not necessary, but it's not harmful, either. > */ > if (psize !=3D MMU_PAGE_4K) > ea &=3D ~((1ul << mmu_psize_defs[psize].shift) - 1); > #endif /* CONFIG_PPC_64K_PAGES */ Yeah, for that one it works (comment needs updating to say that it *is* necessary). I think that's the main thing but there's other possible places where it might not hold -- KVM too, not just the hash refill. > >=20 > > I'd been meaning to do more on this sooner, sorry. I've started > > tinkering with adding a bit of debug code. I'll see if I can help with > > adding a bit of comments. > > Yes would we very welcome, I guess you'll send it as followup/fixup=20 > patch to the series ? Yeah, the basic approach I think is good, so it wouldn't be a big rework. > > >=20 > > [snip] > >=20 > >> diff --git a/arch/powerpc/mm/book3s64/hugetlbpage.c b/arch/powerpc/mm/= book3s64/hugetlbpage.c > >> index 5a2e512e96db..83c3361b358b 100644 > >> --- a/arch/powerpc/mm/book3s64/hugetlbpage.c > >> +++ b/arch/powerpc/mm/book3s64/hugetlbpage.c > >> @@ -53,6 +53,16 @@ int __hash_page_huge(unsigned long ea, unsigned lon= g access, unsigned long vsid, > >> /* If PTE permissions don't match, take page fault */ > >> if (unlikely(!check_pte_access(access, old_pte))) > >> return 1; > >> + /* > >> + * If hash-4k, hugepages use seeral contiguous PxD entries > >> + * so bail out and let mm make the page young or dirty > >> + */ > >> + if (IS_ENABLED(CONFIG_PPC_4K_PAGES)) { > >> + if (!(old_pte & _PAGE_ACCESSED)) > >> + return 1; > >> + if ((access & _PAGE_WRITE) && !(old_pte & _PAGE_DIRTY)) > >> + return 1; > >> + } > >> =20 > >> /* > >> * Try to lock the PTE, add ACCESSED and DIRTY if it was > >=20 > > I'm hoping we wouldn't have to do this, if we follow the PTE[0] rule. > > But we still need all entries to be updated so that page walker which=20 > don't know they must use PTE[0] get the right information ? Ah yeah. Maybe for ACCESSED|DIRTY we can slightly adjust that rule and apply it to all PTEs. If we can do that then it takes care of a few other cases too. Bug what is the consequence of two pte_update racing? Let's say page_vma_mkclean_one vs setting dirty. Can you end up with some PTEs dirty and some not? Thanks, Nick