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 2BFAAC64ED6 for ; Mon, 27 Feb 2023 19:32:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 788B16B0071; Mon, 27 Feb 2023 14:32:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 711C46B0075; Mon, 27 Feb 2023 14:32:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 58B096B0078; Mon, 27 Feb 2023 14:32:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 465D86B0071 for ; Mon, 27 Feb 2023 14:32:13 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1A9FE1C117D for ; Mon, 27 Feb 2023 19:32:13 +0000 (UTC) X-FDA: 80514067746.10.0DD2967 Received: from mail-vs1-f43.google.com (mail-vs1-f43.google.com [209.85.217.43]) by imf28.hostedemail.com (Postfix) with ESMTP id 578E4C0019 for ; Mon, 27 Feb 2023 19:32:11 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=o9jLTfLo; spf=pass (imf28.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.43 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677526331; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=syCwjb07+ozeYYldhWV5SCsjeDRV1y+ehflRxQakiIA=; b=WqRXLttsSYhuN3yE3CRwelLhqHo7Foz898iISCieakA1vQlD76BUq6ioKpOxRljy8vUbGO 7p5H1j7kVdkLdZpdvge7KdAze/h9m2xcsyebyeLLXMxh1xUREc+ZWCYfoBbbI/kPyFCjg1 7xuDqRhwVFBeuaH0aGlaU+67IHOLaXw= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=o9jLTfLo; spf=pass (imf28.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.43 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677526331; a=rsa-sha256; cv=none; b=EQ4CM8QKEykGk22HM0v1nauirFu1N+p0xkLM7+ZGI6qmiJa6CnP0hGL8hqet6vCLtZwcJc wApXAem7dawqIRa5vHF+LiLpOLmVXXJ8s7qQttNj9dRtvcs6ZG4hMMJmgmXWJGG7T8SHc9 yZtBdlaXW/hnAW2waxs0EgxRzQUNXbM= Received: by mail-vs1-f43.google.com with SMTP id s1so12947977vsk.5 for ; Mon, 27 Feb 2023 11:32:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1677526330; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=syCwjb07+ozeYYldhWV5SCsjeDRV1y+ehflRxQakiIA=; b=o9jLTfLoXOAM9KgTs4R8oHGtv9cJ7tkDhFTQBt8Fpct/ccD7ZQ5ZIHpCCd+qk7bsPq yRa490+o1bKJffxsgH0c4B48pCh6iBcK9A4NlMJwkXYemabmiS4Ln2grld1/bxc5sFIT Ch7UTpAUy6Lc7eaiDbPMdaewbZq2kjoeHXu7M+XPzmfEqR7q10SKKhQv/vv0tJlnuxgr kKVdln7RPpO3l6i94NaxAMVqXfNARP/G9Pt/vQwneLjyCHeIoUI5/T+CV7LG/dNpQXVz ZioGH4BMUl5NwDQXCHRIDY6/22D+ebyn2mmMMhLt1Wl0pI1mnxONmohf/x1+tdw9cCXH 00Ig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677526330; h=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=syCwjb07+ozeYYldhWV5SCsjeDRV1y+ehflRxQakiIA=; b=54JgM8cbJi1mc4fywHAyr65a0hfcbFlH+FZ68JA5oYDWJrOjHFSLP8KLKuGwpanFfz ptOjYJFct6ctArbIQajGMjw1MMmojrIlF9UCDsKImJo0K8QyFd7gv5dD4frY1Z3QGhhI 6OcKcdVumt/Ojv2cUn05rToP9BuGdRnot2NgCRkFoeNFAZZ0sqr0X/hZjTECXO+khdrJ laeU9ZS0DFYq5ZvBzgg+kPt4OSD0fpf8BCDm48xnYxjLJ862BPWzkYZzcqM5AD3UNxL/ P6bGsliWQqqYJcjTslVqpKFfCNqomIACxzGVKrtk223hm98vmCS1IXJ6YeAwinP2T/18 Ygbw== X-Gm-Message-State: AO0yUKVBxjlfSWiLre17fFRcGypqKMqjElen2y057Gv4kidNM5DUZd2o 5+kAeLuPjXuzC7SP23gIGMJHHYF4U4ilROp4BSRzQw== X-Google-Smtp-Source: AK7set+upgav+/Kc6aOtPAsemxPPriSwE2AVsQvyawX1VAo7z67icPfFDCi4Pa0MnzmICb7fUS3fKMvNaNuIBs9k1KQ= X-Received: by 2002:a05:6122:1681:b0:401:d1f4:bccf with SMTP id 1-20020a056122168100b00401d1f4bccfmr8803967vkl.0.1677526330331; Mon, 27 Feb 2023 11:32:10 -0800 (PST) MIME-Version: 1.0 References: <20230218002819.1486479-1-jthoughton@google.com> <20230218002819.1486479-13-jthoughton@google.com> In-Reply-To: From: James Houghton Date: Mon, 27 Feb 2023 11:31:33 -0800 Message-ID: Subject: Re: [PATCH v2 12/46] hugetlb: add hugetlb_alloc_pmd and hugetlb_alloc_pte To: Mike Kravetz Cc: Muchun Song , Peter Xu , Andrew Morton , David Hildenbrand , David Rientjes , Axel Rasmussen , Mina Almasry , "Zach O'Keefe" , Manish Mishra , Naoya Horiguchi , "Dr . David Alan Gilbert" , "Matthew Wilcox (Oracle)" , Vlastimil Babka , Baolin Wang , Miaohe Lin , Yang Shi , Frank van der Linden , Jiaqi Yan , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 578E4C0019 X-Stat-Signature: rj1wucn6jwqhukqwi5rwp9wrgukhb8pk X-Rspam-User: X-HE-Tag: 1677526331-739320 X-HE-Meta: U2FsdGVkX19rJgTe152vGwNDejVIbqYGYq97oeDigeLklznB7arP+DYX9rpdkyG7Hf5h6/udBsO+sA54TZp7LtBOCJzLsfWD1Nx0ueE+Fn+uC2ZS0VBxDfzl50+3JhiAQdEXoaD4RqNoRJxRXXAdqTebynPjVfVt7GMLoS80XJ0ULTj6lahFAChlPknhhxUXuL/9ZFj9q7yrX55KgpPEEF2gs4Z+jFVip1z3iyAxA98EZGVFnK/uAlQ2I9i0EyI6FGvDedKZpxCmpD2gA9LgyvEp9+n/w3oaV494lovC9sJY8hJIpRhy+Ae3h1wuZYHim8So+p8i9RhP/2+69Sr4VnqTZyovyhN6MLSOBQb3dnSbSS5ORdna2malpDo6eDCG3rm2hNHpwqvtvCT6m2Ev/Y80lvA/f7NPpn6kDJUdwxv4QerUi4pj3GumwLJlcGX8J7e1lSCvKtd/CMFeQV2rXpQCOANMj0qCI2XclMXGRCm96gIJPhJrxy9orQnvSqGn66C1N257m4SlmC3rDyCdcMy+UrE7JmnRWntR+c/Ba68AZTOw4VgIOX4hwXz55mt2UL8WghidTACc/IMpcpUg1KrdhXRKphxfIbCF/HDtGt/Qzj0JYlNLWq7c8J+AeFj2/6UemrpMsTED21yXF2P+tZxp9PMpPuryq3y+XskMG9D8OHU0JT1knsGWMX70/RfNc5HfBCKv/+1PYhhmThUXHEpwQQRJ2VwRSghTz4KT9lSeSoOg5IlWLzy4FW26jwpHIoQ70yc3xPWRWx+Er4fSg54A1jKzUzSYCXs9xQLR5+ToAVJh5ABcKg2WsCsv1tJQnc96yhl+2T1xdbE1QtceCkqmpa/H+XebJz99nUrr+cfEs+tyLNVhiRf7EPkknYFohxM0J4LM6pihbS/wl8IIkgm/zpPK7YSHHdbdnrcKe58PWqIc1YN//NIhgI8q8uwEtXeDdGIX2J5sJo8qhVN o8ksYQQt dUGezoBWXdZB+vpIfXCF+ylIyF2X4nt7lvG6NfF/VyOXYdhIDg5sBt6hJN2TlbiSaY+olwc55tbCm9mJEB/GE26DDtenKfC1diDJKjKTpcDcwNRj7s3RC7nHjQE8WkOv2yEaEe/0PiVzTLw+TbtNhhvg/EFH82FYrQT3TZutY8QhdKoHyFHcPTAL8QgN8zWgnJW56xnnSzpuPR/Wyp/UJNfFstrVJaAovw7XiQnvsBQgH/W7I6N7t4Pd2DnjWvIKpP2/6nUZt0uxSEQo7YM/kraxTk7iQmqRa6EWNIRoTiL/nqhbCxzqUtzkRlKPHQYsSVALjEk+1MnriFGQn52+z+YaGHA== 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: " On Mon, Feb 27, 2023 at 11:17 AM Mike Kravetz wrote: > > On 02/18/23 00:27, James Houghton wrote: > > These functions are used to allocate new PTEs below the hstate PTE. This > > will be used by hugetlb_walk_step, which implements stepping forwards in > > a HugeTLB high-granularity page table walk. > > > > The reasons that we don't use the standard pmd_alloc/pte_alloc* > > functions are: > > 1) This prevents us from accidentally overwriting swap entries or > > attempting to use swap entries as present non-leaf PTEs (see > > pmd_alloc(); we assume that !pte_none means pte_present and > > non-leaf). > > 2) Locking hugetlb PTEs can different than regular PTEs. (Although, as > > implemented right now, locking is the same.) > > 3) We can maintain compatibility with CONFIG_HIGHPTE. That is, HugeTLB > > HGM won't use HIGHPTE, but the kernel can still be built with it, > > and other mm code will use it. > > > > When GENERAL_HUGETLB supports P4D-based hugepages, we will need to > > implement hugetlb_pud_alloc to implement hugetlb_walk_step. > > > > Signed-off-by: James Houghton > > > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > > index eeacadf3272b..9d839519c875 100644 > > --- a/include/linux/hugetlb.h > > +++ b/include/linux/hugetlb.h > > @@ -72,6 +72,11 @@ unsigned long hugetlb_pte_mask(const struct hugetlb_pte *hpte) > > > > bool hugetlb_pte_present_leaf(const struct hugetlb_pte *hpte, pte_t pte); > > > > +pmd_t *hugetlb_alloc_pmd(struct mm_struct *mm, struct hugetlb_pte *hpte, > > + unsigned long addr); > > +pte_t *hugetlb_alloc_pte(struct mm_struct *mm, struct hugetlb_pte *hpte, > > + unsigned long addr); > > + > > struct hugepage_subpool { > > spinlock_t lock; > > long count; > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index 6c74adff43b6..bb424cdf79e4 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -483,6 +483,120 @@ static bool has_same_uncharge_info(struct file_region *rg, > > #endif > > } > > > > +/* > > + * hugetlb_alloc_pmd -- Allocate or find a PMD beneath a PUD-level hpte. > > + * > > + * This is meant to be used to implement hugetlb_walk_step when one must go to > > + * step down to a PMD. Different architectures may implement hugetlb_walk_step > > + * differently, but hugetlb_alloc_pmd and hugetlb_alloc_pte are architecture- > > + * independent. > > + * > > + * Returns: > > + * On success: the pointer to the PMD. This should be placed into a > > + * hugetlb_pte. @hpte is not changed. > > + * ERR_PTR(-EINVAL): hpte is not PUD-level > > + * ERR_PTR(-EEXIST): there is a non-leaf and non-empty PUD in @hpte > > I often get this confused, should this really be 'non-leaf'? Because, ... This comment is wrong, it should be "non-empty PUD that is not pointing to page tables". Maybe it would be better to say "-EEXIST unless @hpte is pud_none() or already points to page tables". In this commit, PTEs containing PTE markers are treated as non-empty here (and not pointing to page tables), but after the commit "hugetlb: split PTE markers when doing HGM walks", they are treated as empty. I'll update the comment in that commit as well. > > > + * ERR_PTR(-ENOMEM): could not allocate the new PMD > > + */ > > +pmd_t *hugetlb_alloc_pmd(struct mm_struct *mm, struct hugetlb_pte *hpte, > > + unsigned long addr) > > +{ > > + spinlock_t *ptl = hugetlb_pte_lockptr(hpte); > > + pmd_t *new; > > + pud_t *pudp; > > + pud_t pud; > > + > > + if (hpte->level != HUGETLB_LEVEL_PUD) > > + return ERR_PTR(-EINVAL); > > + > > + pudp = (pud_t *)hpte->ptep; > > +retry: > > + pud = READ_ONCE(*pudp); > > + if (likely(pud_present(pud))) > > + return unlikely(pud_leaf(pud)) > > + ? ERR_PTR(-EEXIST) > > + : pmd_offset(pudp, addr); > > ... it seems we return -EEXIST in the pud_leaf case. This code is correct. :) We don't want to overwrite a leaf. Sorry for the confusion!