Received-SPF: pass (google.com: domain of linux-kernel+bounces-32671-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Content-Type: text/plain;
	charset=us-ascii
Precedence: bulk
Mime-Version: 1.0
Subject: Re: [PATCH v4 4/7] hugetlb: pass *next_nid_to_alloc directly to
 for_each_node_mask_to_alloc
From: Muchun Song <muchun.song@linux.dev>
In-Reply-To: <b71d1750-87b9-4f2d-af4a-3555eeffaafd@linux.dev>
Date: Mon, 22 Jan 2024 17:50:26 +0800
Cc: David Hildenbrand <david@redhat.com>,
 David Rientjes <rientjes@google.com>,
 Mike Kravetz <mike.kravetz@oracle.com>,
 Andrew Morton <akpm@linux-foundation.org>,
 Tim Chen <tim.c.chen@linux.intel.com>,
 Linux-MM <linux-mm@kvack.org>,
 LKML <linux-kernel@vger.kernel.org>,
 ligang.bdlg@bytedance.com
Content-Transfer-Encoding: quoted-printable
Message-Id: <84380389-0922-4073-B4C7-DBF9F865E162@linux.dev>
References: <20240118123911.88833-1-gang.li@linux.dev>
 <20240118123911.88833-5-gang.li@linux.dev>
 <f7e072c0-4b41-40d3-bf2e-19d4fdf7bc0d@linux.dev>
 <b71d1750-87b9-4f2d-af4a-3555eeffaafd@linux.dev>
To: Gang Li <gang.li@linux.dev>


> On Jan 22, 2024, at 17:14, Gang Li <gang.li@linux.dev> wrote:
>=20
> On 2024/1/22 14:16, Muchun Song wrote:
>> On 2024/1/18 20:39, Gang Li wrote:
>>>   static struct folio *alloc_pool_huge_folio(struct hstate *h,
>>>                       nodemask_t *nodes_allowed,
>>> -                    nodemask_t *node_alloc_noretry)
>>> +                    nodemask_t *node_alloc_noretry,
>>> +                    int *next_node)
>>>   {
>>>       gfp_t gfp_mask =3D htlb_alloc_mask(h) | __GFP_THISNODE;
>>>       int nr_nodes, node;
>>> -    for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) {
>>> +    for_each_node_mask_to_alloc(next_node, nr_nodes, node, =
nodes_allowed) {
>> A small question here, why not pass h->next_nid_to_alloc to
>> for_each_node_mask_to_alloc()? What's the purpose of the third
>> parameter of alloc_pool_huge_folio()?
>> Thanks.
>=20
> In hugetlb_alloc_node->alloc_pool_huge_folio, hugetlb is initialized =
in
> parallel at boot time, then it needs each thread to have its own
> next_nid, and can't use the global h->next_nid_to_alloc. so an extra =
parameter is added.

Yes. When I read your patch 6, I realized this.

>=20
> And h->next_nid_to_alloc in set_max_huge_pages->alloc_pool_huge_folio
> can not be removed. Because if the user calls set_max_huge_pages
> frequently and only adds 1 page at a time, that would result in each
> request being made on the same node if local variables are used.