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 X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CEE56C433ED for ; Wed, 5 May 2021 22:43:41 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6D406613C0 for ; Wed, 5 May 2021 22:43:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6D406613C0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id EDA326B006C; Wed, 5 May 2021 18:43:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E9ED36B006E; Wed, 5 May 2021 18:43:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D3F546B0070; Wed, 5 May 2021 18:43:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0060.hostedemail.com [216.40.44.60]) by kanga.kvack.org (Postfix) with ESMTP id BAEAE6B006C for ; Wed, 5 May 2021 18:43:40 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 8568E180AD830 for ; Wed, 5 May 2021 22:43:40 +0000 (UTC) X-FDA: 78108655800.19.B18AF76 Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) by imf25.hostedemail.com (Postfix) with ESMTP id 3B48D600010A for ; Wed, 5 May 2021 22:43:33 +0000 (UTC) Received: by mail-ej1-f48.google.com with SMTP id f24so5306464ejc.6 for ; Wed, 05 May 2021 15:43:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Nvaxztmfklo30iHGOAJEqLM9P4mCfCGwm4q3ZK7i77c=; b=UJPAEfLhF+rVabmM7VlnWvBKKsPssvNrRVZmUeyt4UpQXUSyrhTqJSQhT5Z1GJidhR R4iBStO0YbYvC5BW+EME7tGWmVaCjjHwtjU5WnQtofVUBhmoUI3oLmQC66TXR4k0ayYb kwz+S0vPlzYbnIkNc9RZMlpea1Nbmdki0CkHjAAErriVXtoJeyhLiTs7y/iudUm0fjHi mXsK47NC8ntnbqQf7pao3EmfbMCx4UFkRmEKwH/l/txaPS40nFB+MSCewZpvd67NxyJc JaKO9nRYhfAtIwFq/1erh2L+B5RmKnZsw47cqN1O0zRj/gC8rIqnPPJgkRgCtqVdMUKS W5VQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Nvaxztmfklo30iHGOAJEqLM9P4mCfCGwm4q3ZK7i77c=; b=ezy8jBHkx1rBgmBX8IYoIhCM1dpiUtFRvWeiQZ/++1LGb9qStZ2vRu58gTJxNj8Bco XxyTpVxpq2voBXGi3Gt49Yai3z6C4mc2tW1QQp1RjQ2JgRhdttxoYHYuP7n9CzhZIhgI jG8delHPd1gTDQ1PUJ+P18Us1pT4dy1llEGv1choaAhM4TMDossLWo0TN/qD1rMboncc 275CyalsDGFnbb5I0j9AIfIgXLjh+dKVVjGs9zGHjLsmZSVsUO8afbfqedNxSY6Z05H+ LqAYiEmYiBxuhf7coJRi12ZX2plEaHzq8YdO0dIhaiWCuEyB/hH9Kkr/rd6/x/jrdBrZ T1gw== X-Gm-Message-State: AOAM530DVMuwBEVgdueyGpvkgtThX4cphMYGxkzKsDJeXJICg1/tyV2h 2rgfqnosrHLnFfOqx/j3DJeShA/E5b5kqRDb4FcDVA== X-Google-Smtp-Source: ABdhPJyCbCSofL1hxtVo0x0BHI2PTY2fQtKJRbrlNPbsxe1SXgwZusVYhquFtBwiMOhDNyy7b5YByLTaLT3jXhF4j/o= X-Received: by 2002:a17:906:3ec1:: with SMTP id d1mr1020808ejj.523.1620254618614; Wed, 05 May 2021 15:43:38 -0700 (PDT) MIME-Version: 1.0 References: <20210325230938.30752-1-joao.m.martins@oracle.com> <20210325230938.30752-7-joao.m.martins@oracle.com> In-Reply-To: <20210325230938.30752-7-joao.m.martins@oracle.com> From: Dan Williams Date: Wed, 5 May 2021 15:43:43 -0700 Message-ID: Subject: Re: [PATCH v1 06/11] mm/sparse-vmemmap: refactor vmemmap_populate_basepages() To: Joao Martins Cc: Linux MM , Ira Weiny , linux-nvdimm , Matthew Wilcox , Jason Gunthorpe , Jane Chu , Muchun Song , Mike Kravetz , Andrew Morton Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=intel-com.20150623.gappssmtp.com header.s=20150623 header.b=UJPAEfLh; spf=none (imf25.hostedemail.com: domain of dan.j.williams@intel.com has no SPF policy when checking 209.85.218.48) smtp.mailfrom=dan.j.williams@intel.com; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=intel.com (policy=none) X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 3B48D600010A X-Stat-Signature: tbo7tia4a4eriq1edj1i5ry9b84d3jus Received-SPF: none (intel.com>: No applicable sender policy available) receiver=imf25; identity=mailfrom; envelope-from=""; helo=mail-ej1-f48.google.com; client-ip=209.85.218.48 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1620254613-152822 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: I suspect it's a good sign I'm only finding cosmetic and changelog changes in the review... I have some more: A year for now if I'm tracking down a problem and looking through mm commits I would appreciate a subject line like the following: "refactor core of vmemmap_populate_basepages() to helper" that gives an idea of the impact and side effects of the change. On Thu, Mar 25, 2021 at 4:10 PM Joao Martins wrote: > I would add a lead in phrase like: "In preparation for describing a memmap with compound pages, move the actual..." > Move the actual pte population logic into a separate function > vmemmap_populate_address() and have vmemmap_populate_basepages() > walk through all base pages it needs to populate. Aside from the above, looks good. > > Signed-off-by: Joao Martins > --- > mm/sparse-vmemmap.c | 44 ++++++++++++++++++++++++++------------------ > 1 file changed, 26 insertions(+), 18 deletions(-) > > diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c > index 370728c206ee..8814876edcfa 100644 > --- a/mm/sparse-vmemmap.c > +++ b/mm/sparse-vmemmap.c > @@ -216,33 +216,41 @@ pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node) > return pgd; > } > > -int __meminit vmemmap_populate_basepages(unsigned long start, unsigned long end, > - int node, struct vmem_altmap *altmap) > +static int __meminit vmemmap_populate_address(unsigned long addr, int node, > + struct vmem_altmap *altmap) > { > - unsigned long addr = start; > pgd_t *pgd; > p4d_t *p4d; > pud_t *pud; > pmd_t *pmd; > pte_t *pte; > > + pgd = vmemmap_pgd_populate(addr, node); > + if (!pgd) > + return -ENOMEM; > + p4d = vmemmap_p4d_populate(pgd, addr, node); > + if (!p4d) > + return -ENOMEM; > + pud = vmemmap_pud_populate(p4d, addr, node); > + if (!pud) > + return -ENOMEM; > + pmd = vmemmap_pmd_populate(pud, addr, node); > + if (!pmd) > + return -ENOMEM; > + pte = vmemmap_pte_populate(pmd, addr, node, altmap); > + if (!pte) > + return -ENOMEM; > + vmemmap_verify(pte, node, addr, addr + PAGE_SIZE); > +} > + > +int __meminit vmemmap_populate_basepages(unsigned long start, unsigned long end, > + int node, struct vmem_altmap *altmap) > +{ > + unsigned long addr = start; > + > for (; addr < end; addr += PAGE_SIZE) { > - pgd = vmemmap_pgd_populate(addr, node); > - if (!pgd) > - return -ENOMEM; > - p4d = vmemmap_p4d_populate(pgd, addr, node); > - if (!p4d) > - return -ENOMEM; > - pud = vmemmap_pud_populate(p4d, addr, node); > - if (!pud) > - return -ENOMEM; > - pmd = vmemmap_pmd_populate(pud, addr, node); > - if (!pmd) > - return -ENOMEM; > - pte = vmemmap_pte_populate(pmd, addr, node, altmap); > - if (!pte) > + if (vmemmap_populate_address(addr, node, altmap)) > return -ENOMEM; > - vmemmap_verify(pte, node, addr, addr + PAGE_SIZE); > } > > return 0; > -- > 2.17.1 >