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 7F3CDC61DB3 for ; Thu, 12 Jan 2023 13:38:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9F8EE8E0002; Thu, 12 Jan 2023 08:38:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9A8618E0001; Thu, 12 Jan 2023 08:38:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8721C8E0002; Thu, 12 Jan 2023 08:38:18 -0500 (EST) 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 7714D8E0001 for ; Thu, 12 Jan 2023 08:38:18 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 36097AB302 for ; Thu, 12 Jan 2023 13:38:18 +0000 (UTC) X-FDA: 80346251076.25.B20DF8F Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by imf16.hostedemail.com (Postfix) with ESMTP id 5B714180019 for ; Thu, 12 Jan 2023 13:38:15 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Xp0TRmql; spf=pass (imf16.hostedemail.com: domain of jthoughton@google.com designates 209.85.128.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=1673530695; a=rsa-sha256; cv=none; b=fJ4NR+iqynRll4eXCuD4q0QiGckn0cVSZ7ju+oOK/sI8cFVOxpLZlPqFhX4WD9rA6RdId3 62cWmatGxzw1YRMPoQhQQibQe7YfUh5lRosxoILcO295/4zolpiz3Ry6ewrUpgCm8AcFYX 4wD3vwUxSVl5WrUAFeIBYBGSvLtnL/U= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Xp0TRmql; spf=pass (imf16.hostedemail.com: domain of jthoughton@google.com designates 209.85.128.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=1673530695; 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=znmW8joeNg1jmfKypKeLrmbUtyOkQqXYCi/lvfTsrdI=; b=a0Dh/+1WwASeiJdNXPfIdarA17kcwcbahIAMdeuIGtwyGJNNHKdbQrTHKSJV6xRaskQq2q hcDGLjuqXpBP1gmbHv7q58SKld113fXUH5sGyIK4vI94vlRX59UnAzBltxUo68ydgcQKWY ZtoDVgNj1IIH5oD/exvMIwShthyRrzo= Received: by mail-wm1-f43.google.com with SMTP id q8so1407897wmo.5 for ; Thu, 12 Jan 2023 05:38:15 -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=znmW8joeNg1jmfKypKeLrmbUtyOkQqXYCi/lvfTsrdI=; b=Xp0TRmqlsjjffiwxl0b0CiwXPilusQSP/SU9BxYBpigVwWpqBMplvDmS+3MSAPF09i PTr+3OYQuzOIR2mg7EpxH5tn4S85J7qF4RzNWLrVnLcFc+CSEWWEY12VX5+rVcBvYRnM KoGadlfGXw9Z8Uk37XSlBLxEw3uq0BkmE7/ZxPkROFbpUW+tXU70hKkK4b97ajVSex+g LbFJdoYgaY5d5QhwBBVp1EJmT5KyRLATPVP3M0D97hKFNKdjX5p47Ky/OQncckodt17q egXGj09/aeLOdwwY6xKZssRm/yHPlWyWIaNoWXlHdXfTmQh/PrPr3jatx/kzw+AshjGJ WAwQ== 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=znmW8joeNg1jmfKypKeLrmbUtyOkQqXYCi/lvfTsrdI=; b=v+hyk1Y/iQ9dJGQb+sfsXi6ThCEU8Sgwv4KZa2jyrEUS8UajVKULWSed0Ok/c1SGhb 9D1zM70tEDfBtrnv3qwgyOE0UtLIfPQc4BcaknvUacG2ZmDZ+wwUtmkhBBRLLJSALc0E HVyj5miif1r0fLDr7sr5tIZzC1XMV7PgxuB/qVdseS17NU6KSEMckLw6kAqJH+HhSv2/ tLFu0JUfvWoqVtab1HSUZQeu/Muu7XSRfC4lLUTRzHZ0rol1EIQCKlICXKQmOUJT0dCX Qz4sWbqa4gy8QgB1BOQsA0cv6U6IuhTti3iYYjBziAjr7hOIbzbs2Wb5igNOyVWdtiTd BXqg== X-Gm-Message-State: AFqh2koPFoDKKdmG8s10P071yDYkH2yJN8M52I+6Brt0Q1q6RLjZ+Ugq qoeP6V5cShFWNNh9ni7880qFTZABpJK0fGnP8Evk8A== X-Google-Smtp-Source: AMrXdXvsTW5fSCQd5wEYlEg/EKrAsjeKT1Gel7lICz1kX2DxrngJOgXMxeF4KdV/Unu34j7CRdbSeHop2BZO6p2yeL8= X-Received: by 2002:a05:600c:1684:b0:3da:1b37:8ff5 with SMTP id k4-20020a05600c168400b003da1b378ff5mr76464wmn.166.1673530693828; Thu, 12 Jan 2023 05:38:13 -0800 (PST) MIME-Version: 1.0 References: <20230105101844.1893104-1-jthoughton@google.com> <20230105101844.1893104-14-jthoughton@google.com> In-Reply-To: From: James Houghton Date: Thu, 12 Jan 2023 08:38:02 -0500 Message-ID: Subject: Re: [PATCH 13/46] hugetlb: add hugetlb_hgm_walk and hugetlb_walk_step To: Peter Xu Cc: Mike Kravetz , Muchun Song , 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 , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Queue-Id: 5B714180019 X-Rspamd-Server: rspam01 X-Stat-Signature: hqn7t5ybmpoti7rgo4iighr138ab3xmc X-HE-Tag: 1673530695-919611 X-HE-Meta: U2FsdGVkX1+ZJejY0o/d+PxcONQfs2KFr0WXVCE3yqqF10CJT52rakfjePY2P1MwYevwjY+2VvPdNg8Iloy+t58H+YA3vUHXJoWAE/btFcQF6qe+J2XvGzgjHkVQgs4fTyHgFGZ1hDiIo6GR0HHQt9vdrzxbOVvIhLUjbydcnft1F3W87baWoY5uq7NeIc8ey+a+NY5iQn7iKsyAToF+P8XAJTJmzNffGt9+laWxPMEPGfdbCe12yEvpoA7yP655JUr2Yl602RLX0JtboRVwCSIga+eqq764ysw9kmZLKoAgUTvORfW6WdPILK8U8qFMwWpZ15BW13LVDZeaBAeHxt6jW7Q75UgGFctgqs5qAw/9J+B6Yr9tckC8Ca3DLsHz/Ai9bsdUT1ZJVPdKWsykmLsarc7H8Z3QNKA2R3xXEAs9u0X8j3qlozgl9tSuwUUk2HPbdMQhScQbyiQpq75wKqx4hPh/FAYoCnsbZ+eig/fkZkC3wJ+H0LSJaLWmdTP7cA2y74LQCLcd7XezqEZJWA9xekVLKnhsiACetlehC9JM38oHQJtbSU4xV4HVKVbxd5y01Zv0/WTsD2Ab44b4mBekpkcXveMolYjM0BKISRx4L8jiQcNAoCvtN/Bh6ZacDzZtY6rx7IPGhiGzQLUgEwKzVHtcjT74esTPWrogCG59uTC3YeCFbGdkki75TEgO3rNYPBnaSdHNyDrNXNdlCbhFw+TI67ElC13GrZk42MozbnRjzz5kHQN0AraoXni4klrHSpNYWznmlBB7pQlr0oHcApJTp/V26OY6DahbMtIGrw0Z25RZ92rg6SV3XfNAITEOfH7w3WohUa46WKEfqcFUg7PdpmtfOsXssIgeSFH9DTk36uzi5OZ6/6FnhizqTbMG2C8CeFJ0cPpqU+3MvE3QFQe/AYc1oa2xvUyRFbgWBTnH14q1eDqnmkFKeWsYq+0VaGHX7M/Y+SAPMCK CzRHE72m t7pP1BiXF3Suc2wtDrHQ67Ct5Meiwm83xc5vlGdnqPjJETGNkv484lTI+jg== 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 Wed, Jan 11, 2023 at 4:51 PM Peter Xu wrote: > > On Thu, Jan 05, 2023 at 10:18:11AM +0000, James Houghton wrote: > > [...] > > > +static int hugetlb_hgm_walk_uninit(struct hugetlb_pte *hpte, > > Nitpick on the name: the "uninit" can be misread into pairing with some > other "init()" calls.. > > How about just call it hugetlb_hgm_walk (since it's the higher level API > comparing to the existing one)? Then the existing hugetlb_hgm_walk can be > called hugetlb_hgm_do_walk/__hugetlb_hgm_walk since it's one level down. > > > + pte_t *ptep, > > + struct vm_area_struct *vma, > > + unsigned long addr, > > + unsigned long target_sz, > > + bool alloc) > > +{ > > + struct hstate *h = hstate_vma(vma); > > + > > + hugetlb_pte_populate(vma->vm_mm, hpte, ptep, huge_page_shift(h), > > + hpage_size_to_level(huge_page_size(h))); > > Another nitpick on name: I remembered we used to reach a consensus of using > hugetlb_pte_init before? Can we still avoid the word "populate" (if "init" > is not suitable since it can be updated during stepping, how about "setup")? Right, we did talk about this, sorry. Ok I'll go ahead with this name change. - hugetlb_hgm_walk => __hugetlb_hgm_walk - hugetlb_hgm_walk_uninit => hugetlb_hgm_walk - [__,]hugetlb_pte_populate => [__,]hugetlb_pte_init > > [...] > > > +int hugetlb_walk_step(struct mm_struct *mm, struct hugetlb_pte *hpte, > > + unsigned long addr, unsigned long sz) > > +{ > > + pte_t *ptep; > > + spinlock_t *ptl; > > + > > + switch (hpte->level) { > > + case HUGETLB_LEVEL_PUD: > > + ptep = (pte_t *)hugetlb_alloc_pmd(mm, hpte, addr); > > + if (IS_ERR(ptep)) > > + return PTR_ERR(ptep); > > + hugetlb_pte_populate(mm, hpte, ptep, PMD_SHIFT, > > + HUGETLB_LEVEL_PMD); > > + break; > > + case HUGETLB_LEVEL_PMD: > > + ptep = hugetlb_alloc_pte(mm, hpte, addr); > > + if (IS_ERR(ptep)) > > + return PTR_ERR(ptep); > > + ptl = pte_lockptr(mm, (pmd_t *)hpte->ptep); > > + __hugetlb_pte_populate(hpte, ptep, PAGE_SHIFT, > > + HUGETLB_LEVEL_PTE, ptl); > > + hpte->ptl = ptl; > > This line seems to be superfluous (even if benign). Nice catch! It shouldn't be there; I accidentally left it in when I changed how `ptl` was handled. Thanks Peter!