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 49CBDC4708D for ; Thu, 8 Dec 2022 00:46:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C591F8E0003; Wed, 7 Dec 2022 19:46:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C09058E0001; Wed, 7 Dec 2022 19:46:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD10A8E0003; Wed, 7 Dec 2022 19:46:30 -0500 (EST) 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 9828D8E0001 for ; Wed, 7 Dec 2022 19:46:30 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 6713BA0EED for ; Thu, 8 Dec 2022 00:46:30 +0000 (UTC) X-FDA: 80217298140.14.2381ED5 Received: from mail-vs1-f54.google.com (mail-vs1-f54.google.com [209.85.217.54]) by imf26.hostedemail.com (Postfix) with ESMTP id DD3F9140005 for ; Thu, 8 Dec 2022 00:46:28 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=YCoigiVi; spf=pass (imf26.hostedemail.com: domain of almasrymina@google.com designates 209.85.217.54 as permitted sender) smtp.mailfrom=almasrymina@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670460388; a=rsa-sha256; cv=none; b=Y8sUIxQsiwwC3aOy/uuEgQ7hUbO1JwxuENpQ7iSIZt5pfHqN82YcGajEWy229j4FLYIz4t 5PNR4MJ8rwYQNMASmz0VUI/a0ROUaHPnuk7Uea9pGhDaXoXw6r9tXeK+pQ2v6EjXnwIS8m VdfF7951alqHk3slPpxXn0IU8A4GbQk= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=YCoigiVi; spf=pass (imf26.hostedemail.com: domain of almasrymina@google.com designates 209.85.217.54 as permitted sender) smtp.mailfrom=almasrymina@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=1670460388; 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=BIw7NaxMlqZRuJd3JvVxKniDE1qFghGLpAfWUnuQmyg=; b=p/VBQF6i24g9tZIYrF2gVC5RVDRaC/QXKvvVsih86IWk2SdVq+eSedkBsAQUUTyDzSmCiP QLfJBiTXAUKSGekdHljHNq4wjL+tdTLGh6MpZ2XLZj11nakvg82iQyZRVJI+6IjzgBqigr QyK4rlOhYjNHDmeEALmd84IzyLtcNpE= Received: by mail-vs1-f54.google.com with SMTP id b189so94451vsc.10 for ; Wed, 07 Dec 2022 16:46:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=BIw7NaxMlqZRuJd3JvVxKniDE1qFghGLpAfWUnuQmyg=; b=YCoigiVijSXgi0g70gvipylQr8/o8f9/vgZiOrAK5bsQfAjlyalnETlwr4oBqWO4Kp 6oA/ptR/EqYj39I7qdWoFvCqBqJLV0O1emBjpnpVj6TEQThTp9PcqjYZCiDQ6wXIthh6 SjQ5JlipNTDsWndi9LmBObgrFWXCuQLsfwcxsYMhUOjtQ8PTaeGCyhDSWBiRaSrbCUIz b0xNKgQTuYBZLUUEizRaJLGts0XrM/swCyuElCvfQQfXkrul7+P/e+WDMt9sAw4DJNhz RLdfVsAUBWlzFpHL6b0RcQi98T6wZAXsklAZ9qiuW/wLVxMwYb+sJwnhFlPB9yIvcbtw +NRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=BIw7NaxMlqZRuJd3JvVxKniDE1qFghGLpAfWUnuQmyg=; b=TnoEzMwEWkcKMkFWTB0/W7Nys7Jz4RpwQx71pTb58ibJcVGApQAi1EWwJhrFRS/Mf+ cZ15Zw/DUwLLgTQ/F3NQrtxskKHsHJocOzeVCK/TrjcPuoCbdFt/lAtohC3+0dTw1kHR gyZ574IzQYBojwFc/KYulxJWZGzGEvFz4hsQYPNmBJVhpdcBd8ftWUutmPoslxC/jomO M3RryZC2QTd+oHhzB5r9b4riKEUkqFJnBhff59tsoKLQX3eNnosS6OvyWNlgUIz/++Ai CFhGygGcpx1kIsqUJY2SautYELGr+zOvqPmn/Yr6c9wpjnYXak97zFTvMBbvWSR6yEfZ eLaQ== X-Gm-Message-State: ANoB5pmtKHmt0AcqB0CPHdUIqFR01h+qgWnEcL8VgHrdiZYYOkxbjTyj 0hSlJCDXWtYV27SkY7Qjb7FvZLP8XKVzX3R/2xYuFQ== X-Google-Smtp-Source: AA0mqf7hjK9bZqtIrLbR1QFV8hkV8UwY43wqwiULUFLce+43tiZ2SqJh9fo4wj57lFTP4ztvMAFp1g1lqg1NSFBzYjc= X-Received: by 2002:a05:6102:2758:b0:3b1:1962:24f9 with SMTP id p24-20020a056102275800b003b1196224f9mr12696662vsu.72.1670460387885; Wed, 07 Dec 2022 16:46:27 -0800 (PST) MIME-Version: 1.0 References: <20221021163703.3218176-1-jthoughton@google.com> <20221021163703.3218176-11-jthoughton@google.com> In-Reply-To: <20221021163703.3218176-11-jthoughton@google.com> From: Mina Almasry Date: Wed, 7 Dec 2022 16:46:16 -0800 Message-ID: Subject: Re: [RFC PATCH v2 10/47] hugetlb: add hugetlb_pte to track HugeTLB page table entries To: James Houghton Cc: Mike Kravetz , Muchun Song , Peter Xu , David Hildenbrand , David Rientjes , Axel Rasmussen , "Zach O'Keefe" , Manish Mishra , Naoya Horiguchi , "Dr . David Alan Gilbert" , "Matthew Wilcox (Oracle)" , Vlastimil Babka , Baolin Wang , Miaohe Lin , Yang Shi , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: DD3F9140005 X-Stat-Signature: pfy6s4px8pu6hdhwoccii4qyk1dgns8u X-HE-Tag: 1670460388-512520 X-HE-Meta: U2FsdGVkX1+dqaOGgDQGEOIZBDj5y5QrrRBfx06WhSJPWCdCTyU4EPO+Dy+HJtH7DYmbZhSeA9+clBeLLGKRwbIjbW3eUeRSH0Dciq9nQzcyc+uQ7ZZfhwK9TRNGITBiP7Yzuow5Q9DKxFjTg01N9Xb5vSC56O1+59WfVNbuMJsANGypmVTJflLxT3wLtWG6UDscBiE99BobWLAogF80M0rhV33J8kXuodQNdj93gOmsTCKNvxSbDQ+kNe+Uqrdra6CH0++mS6/TpTTyiuGVaOlx2IN874S6XgsIBb1UXuBVcHpDQvbkVaiSnCLnbdb+tw0OUqs2ERPDgaEoUqyGIM0D8aTVqwncTYoQ1V1TY2e5Lbbdwrs//euKxSPBSiz7GY4o9Vldkp6QRIHY7iPJn6QypY4fkm1cnUyd1pqgfH0lYJ/rjECz7lh13nujKz8Qs/GBdhOV1e8noqwUn7z4kqFitM76H8oXpmMPVJpIrn1IIooJfmRonTMsXt0eCcBWvSljcTXK6jx7EU7Fzt7ULm6yqVRCLcHsm2EGE9hp2TbUcS0sruDt5PrRbRmmDQmD/deYUi/F+zdUxuzWUQ4az1Zu10n52BiCgQjaOfJ4FfR9yS7lgaom0bU4V5Xn8jBu3J1h/aQHB69Z+QVyoNHt0n0zTddzSVKBK1p3kG3vjV2Vtzs5SsjVDAzk7jrNJ82OuyXBTkOGggcHqH1wrjzp5+Eh5LeyE85Ln50H1UkM92nIWXron6z2IlQ8nbYQ+FAFbrQ2L9hW3wzq8lj+9LUCbRCCB4iA+Z2gVrfdklMffgQGAOF4WdzCaLMFAp69OTVxYb/BGXIP7gbo9PqUPyZL7TCMYtoDaAR5BPdI0cYNhbdVEE/yx5Lpb6RO1/B0kwKQESPIHRUUPplThLuX+hPFcb6fSMvpvqb5TRFCopqbKYuREPxJTPkf6CviwVlh08ZHqIhT95TNGFViMXQXeOg 5nnz+ZDH TFHcC3GFSEvlIfB61+ekY4GLKh6tcLJVS0eRtwHO7nPzN0+EopOk5LK5+Nbl4pHVN6PY8 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 Fri, Oct 21, 2022 at 9:37 AM James Houghton wrote: > > After high-granularity mapping, page table entries for HugeTLB pages can > be of any size/type. (For example, we can have a 1G page mapped with a > mix of PMDs and PTEs.) This struct is to help keep track of a HugeTLB > PTE after we have done a page table walk. > > Without this, we'd have to pass around the "size" of the PTE everywhere. > We effectively did this before; it could be fetched from the hstate, > which we pass around pretty much everywhere. > > hugetlb_pte_present_leaf is included here as a helper function that will > be used frequently later on. > > Signed-off-by: James Houghton > --- > include/linux/hugetlb.h | 88 +++++++++++++++++++++++++++++++++++++++++ > mm/hugetlb.c | 29 ++++++++++++++ > 2 files changed, 117 insertions(+) > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > index db3ed6095b1c..d30322108b34 100644 > --- a/include/linux/hugetlb.h > +++ b/include/linux/hugetlb.h > @@ -50,6 +50,75 @@ enum { > __NR_USED_SUBPAGE, > }; > > +enum hugetlb_level { > + HUGETLB_LEVEL_PTE = 1, > + /* > + * We always include PMD, PUD, and P4D in this enum definition so that, > + * when logged as an integer, we can easily tell which level it is. > + */ > + HUGETLB_LEVEL_PMD, > + HUGETLB_LEVEL_PUD, > + HUGETLB_LEVEL_P4D, > + HUGETLB_LEVEL_PGD, > +}; > + Don't we need to support CONTIG_PTE/PMD levels here for ARM64? > +struct hugetlb_pte { > + pte_t *ptep; > + unsigned int shift; > + enum hugetlb_level level; Is shift + level redundant? When would those diverge? > + spinlock_t *ptl; > +}; > + > +static inline > +void hugetlb_pte_populate(struct hugetlb_pte *hpte, pte_t *ptep, > + unsigned int shift, enum hugetlb_level level) > +{ > + WARN_ON_ONCE(!ptep); > + hpte->ptep = ptep; > + hpte->shift = shift; > + hpte->level = level; > + hpte->ptl = NULL; > +} > + > +static inline > +unsigned long hugetlb_pte_size(const struct hugetlb_pte *hpte) > +{ > + WARN_ON_ONCE(!hpte->ptep); > + return 1UL << hpte->shift; > +} > + > +static inline > +unsigned long hugetlb_pte_mask(const struct hugetlb_pte *hpte) > +{ > + WARN_ON_ONCE(!hpte->ptep); > + return ~(hugetlb_pte_size(hpte) - 1); > +} > + > +static inline > +unsigned int hugetlb_pte_shift(const struct hugetlb_pte *hpte) > +{ > + WARN_ON_ONCE(!hpte->ptep); > + return hpte->shift; > +} > + > +static inline > +enum hugetlb_level hugetlb_pte_level(const struct hugetlb_pte *hpte) > +{ > + WARN_ON_ONCE(!hpte->ptep); > + return hpte->level; > +} > + > +static inline > +void hugetlb_pte_copy(struct hugetlb_pte *dest, const struct hugetlb_pte *src) > +{ > + dest->ptep = src->ptep; > + dest->shift = src->shift; > + dest->level = src->level; > + dest->ptl = src->ptl; > +} > + > +bool hugetlb_pte_present_leaf(const struct hugetlb_pte *hpte, pte_t pte); > + > struct hugepage_subpool { > spinlock_t lock; > long count; > @@ -1210,6 +1279,25 @@ static inline spinlock_t *huge_pte_lock(struct hstate *h, > return ptl; > } > > +static inline > +spinlock_t *hugetlb_pte_lockptr(struct mm_struct *mm, struct hugetlb_pte *hpte) > +{ > + > + BUG_ON(!hpte->ptep); I think BUG_ON()s will be frowned upon. This function also doesn't really need ptep. Maybe let hugetlb_pte_shift() decide to BUG_ON() if necessary. > + if (hpte->ptl) > + return hpte->ptl; > + return huge_pte_lockptr(hugetlb_pte_shift(hpte), mm, hpte->ptep); I don't know if this fallback to huge_pte_lockptr() should be obivous to the reader. If not, a comment would help. > +} > + > +static inline > +spinlock_t *hugetlb_pte_lock(struct mm_struct *mm, struct hugetlb_pte *hpte) > +{ > + spinlock_t *ptl = hugetlb_pte_lockptr(mm, hpte); > + > + spin_lock(ptl); > + return ptl; > +} > + > #if defined(CONFIG_HUGETLB_PAGE) && defined(CONFIG_CMA) > extern void __init hugetlb_cma_reserve(int order); > #else > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index ef7662bd0068..a0e46d35dabc 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1127,6 +1127,35 @@ static bool vma_has_reserves(struct vm_area_struct *vma, long chg) > return false; > } > > +bool hugetlb_pte_present_leaf(const struct hugetlb_pte *hpte, pte_t pte) I also don't know if this is obvious to other readers, but I'm quite confused that we pass both hugetlb_pte and pte_t here, especially when hpte has a pte_t inside of it. Maybe a comment would help. > +{ > + pgd_t pgd; > + p4d_t p4d; > + pud_t pud; > + pmd_t pmd; > + > + WARN_ON_ONCE(!hpte->ptep); > + switch (hugetlb_pte_level(hpte)) { > + case HUGETLB_LEVEL_PGD: > + pgd = __pgd(pte_val(pte)); > + return pgd_present(pgd) && pgd_leaf(pgd); > + case HUGETLB_LEVEL_P4D: > + p4d = __p4d(pte_val(pte)); > + return p4d_present(p4d) && p4d_leaf(p4d); > + case HUGETLB_LEVEL_PUD: > + pud = __pud(pte_val(pte)); > + return pud_present(pud) && pud_leaf(pud); > + case HUGETLB_LEVEL_PMD: > + pmd = __pmd(pte_val(pte)); > + return pmd_present(pmd) && pmd_leaf(pmd); > + case HUGETLB_LEVEL_PTE: > + return pte_present(pte); > + default: > + WARN_ON_ONCE(1); > + return false; > + } > +} > + > static void enqueue_huge_page(struct hstate *h, struct page *page) > { > int nid = page_to_nid(page); > -- > 2.38.0.135.g90850a2211-goog >