* [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 [PATCH v3 0/4] hugetlb: Fix some incorrect behavior Peng Liu
@ 2022-04-13 3:29 ` Peng Liu
2022-04-13 4:42 ` Andrew Morton
` (4 more replies)
2022-04-13 3:29 ` [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode Peng Liu
` (2 subsequent siblings)
3 siblings, 5 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-13 3:29 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, liupeng256
Certain systems are designed to have sparse/discontiguous nodes. In
this case, nr_online_nodes can not be used to walk through numa node.
Also, a valid node may be greater than nr_online_nodes.
However, in hugetlb, it is assumed that nodes are contiguous. Recheck
all the places that use nr_online_nodes, and repair them one by one.
Suggested-by: David Hildenbrand <david@redhat.com>
Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
mm/hugetlb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b34f50156f7e..5b5a2a5a742f 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate *h, int nid)
struct huge_bootmem_page *m = NULL; /* initialize for clang */
int nr_nodes, node;
- if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
+ if (nid != NUMA_NO_NODE && !node_online(nid))
return 0;
/* do node specific alloc */
if (nid != NUMA_NO_NODE) {
@@ -3088,7 +3088,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
}
/* do node specific alloc */
- for (i = 0; i < nr_online_nodes; i++) {
+ for_each_online_node(i) {
if (h->max_huge_pages_node[i] > 0) {
hugetlb_hstate_alloc_pages_onenode(h, i);
node_specific_alloc = true;
@@ -4049,7 +4049,7 @@ static int __init hugetlb_init(void)
default_hstate.max_huge_pages =
default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
}
@@ -4164,9 +4164,9 @@ static int __init hugepages_setup(char *s)
pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
return 0;
}
- if (tmp >= nr_online_nodes)
+ if (!node_online(tmp))
goto invalid;
- node = array_index_nospec(tmp, nr_online_nodes);
+ node = array_index_nospec(tmp, MAX_NUMNODES);
p += count + 1;
/* Parse hugepages */
if (sscanf(p, "%lu%n", &tmp, &count) != 1)
@@ -4294,7 +4294,7 @@ static int __init default_hugepagesz_setup(char *s)
*/
if (default_hstate_max_huge_pages) {
default_hstate.max_huge_pages = default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
if (hstate_is_gigantic(&default_hstate))
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
@ 2022-04-13 4:42 ` Andrew Morton
[not found] ` <692ee24c-a705-0c54-7cad-a9ecf49a8f15@huawei.com>
2022-04-13 6:29 ` Baolin Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2022-04-13 4:42 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On Wed, 13 Apr 2022 03:29:12 +0000 Peng Liu <liupeng256@huawei.com> wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
>
What are the runtime effects of this shortcoming?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
2022-04-13 4:42 ` Andrew Morton
@ 2022-04-13 6:29 ` Baolin Wang
2022-04-14 23:36 ` Mike Kravetz
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Baolin Wang @ 2022-04-13 6:29 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/13/2022 11:29 AM, Peng Liu wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
2022-04-13 4:42 ` Andrew Morton
2022-04-13 6:29 ` Baolin Wang
@ 2022-04-14 23:36 ` Mike Kravetz
2022-04-15 2:09 ` Davidlohr Bueso
2022-04-16 10:35 ` [PATCH v4] " Peng Liu
4 siblings, 0 replies; 37+ messages in thread
From: Mike Kravetz @ 2022-04-14 23:36 UTC (permalink / raw)
To: Peng Liu, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/12/22 20:29, Peng Liu wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Thank you!
I am guessing that at one time nodes were contiguous at least at boot time.
When that changed, hugetlb was not updated. :(
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
` (2 preceding siblings ...)
2022-04-14 23:36 ` Mike Kravetz
@ 2022-04-15 2:09 ` Davidlohr Bueso
2022-04-15 5:41 ` Kefeng Wang
2022-04-16 10:35 ` [PATCH v4] " Peng Liu
4 siblings, 1 reply; 37+ messages in thread
From: Davidlohr Bueso @ 2022-04-15 2:09 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On Wed, 13 Apr 2022, Peng Liu wrote:
>Certain systems are designed to have sparse/discontiguous nodes. In
>this case, nr_online_nodes can not be used to walk through numa node.
>Also, a valid node may be greater than nr_online_nodes.
>
>However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>all the places that use nr_online_nodes, and repair them one by one.
>
>Suggested-by: David Hildenbrand <david@redhat.com>
>Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
>Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
>Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
>Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
>Signed-off-by: Peng Liu <liupeng256@huawei.com>
>Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
... but
>---
> mm/hugetlb.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>index b34f50156f7e..5b5a2a5a742f 100644
>--- a/mm/hugetlb.c
>+++ b/mm/hugetlb.c
>@@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate *h, int nid)
> struct huge_bootmem_page *m = NULL; /* initialize for clang */
> int nr_nodes, node;
>
>- if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
>+ if (nid != NUMA_NO_NODE && !node_online(nid))
afaict null_blk could also use this, actually the whole thing wants a
helper - node_valid()?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-15 2:09 ` Davidlohr Bueso
@ 2022-04-15 5:41 ` Kefeng Wang
2022-04-16 1:21 ` Kefeng Wang
0 siblings, 1 reply; 37+ messages in thread
From: Kefeng Wang @ 2022-04-15 5:41 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel, Kefeng Wang
On 2022/4/15 10:09, Davidlohr Bueso wrote:
> On Wed, 13 Apr 2022, Peng Liu wrote:
>
>> Certain systems are designed to have sparse/discontiguous nodes. In
>> this case, nr_online_nodes can not be used to walk through numa node.
>> Also, a valid node may be greater than nr_online_nodes.
>>
>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>> all the places that use nr_online_nodes, and repair them one by one.
>>
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of
>> gigantic pages can't work")
>> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages
>> parameter to support node allocation")
>> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages
>> parameter")
>> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue
>> warnings")
>> Signed-off-by: Peng Liu <liupeng256@huawei.com>
>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>
> ... but
>
>> ---
>> mm/hugetlb.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index b34f50156f7e..5b5a2a5a742f 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate *h,
>> int nid)
>> struct huge_bootmem_page *m = NULL; /* initialize for clang */
>> int nr_nodes, node;
>>
>> - if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
>> + if (nid != NUMA_NO_NODE && !node_online(nid))
>
> afaict null_blk could also use this, actually the whole thing wants a
> helper - node_valid()?
>
This one should be unnecessary, and this patch looks has a bug,
if a very nid passed to node_online(), it may crash, could you re-check
it,
see my changes below,
1) add tmp check against MAX_NUMNODES before node_online() check,
and move it after get tmp in hugepages_setup() , this could cover
both per-node alloc and normal alloc
2) due to for_each_online_node() usage, we can drop additional check of
nid in __alloc_bootmem_huge_page()
$ git diff
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index fb5a549169ce..5a3ddec181a0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2986,8 +2986,6 @@ int __alloc_bootmem_huge_page(struct hstate *h,
int nid)
struct huge_bootmem_page *m = NULL; /* initialize for clang */
int nr_nodes, node;
- if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
- return 0;
/* do node specific alloc */
if (nid != NUMA_NO_NODE) {
m = memblock_alloc_try_nid_raw(huge_page_size(h),
huge_page_size(h),
@@ -3095,7 +3093,7 @@ static void __init
hugetlb_hstate_alloc_pages(struct hstate *h)
}
/* do node specific alloc */
- for (i = 0; i < nr_online_nodes; i++) {
+ for_each_online_node(i) {
if (h->max_huge_pages_node[i] > 0) {
hugetlb_hstate_alloc_pages_onenode(h, i);
node_specific_alloc = true;
@@ -4059,7 +4057,7 @@ static int __init hugetlb_init(void)
default_hstate.max_huge_pages =
default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
}
@@ -4168,15 +4166,15 @@ static int __init hugepages_setup(char *s)
count = 0;
if (sscanf(p, "%lu%n", &tmp, &count) != 1)
goto invalid;
+ if (tmp > MAX_NUMNODES || !node_online(tmp))
+ goto invalid;
/* Parameter is node format */
if (p[count] == ':') {
if (!hugetlb_node_alloc_supported()) {
pr_warn("HugeTLB: architecture can't
support node specific alloc, ignoring!\n");
return 0;
}
- if (tmp >= nr_online_nodes)
- goto invalid;
- node = array_index_nospec(tmp, nr_online_nodes);
+ node = array_index_nospec(tmp, MAX_NUMNODES);
p += count + 1;
/* Parse hugepages */
if (sscanf(p, "%lu%n", &tmp, &count) != 1)
@@ -4304,7 +4302,7 @@ static int __init default_hugepagesz_setup(char *s)
*/
if (default_hstate_max_huge_pages) {
default_hstate.max_huge_pages =
default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
if (hstate_is_gigantic(&default_hstate))
> .
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-15 5:41 ` Kefeng Wang
@ 2022-04-16 1:21 ` Kefeng Wang
2022-04-19 4:40 ` Andrew Morton
0 siblings, 1 reply; 37+ messages in thread
From: Kefeng Wang @ 2022-04-16 1:21 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel
On 2022/4/15 13:41, Kefeng Wang wrote:
>
> On 2022/4/15 10:09, Davidlohr Bueso wrote:
>> On Wed, 13 Apr 2022, Peng Liu wrote:
>>
>>> Certain systems are designed to have sparse/discontiguous nodes. In
>>> this case, nr_online_nodes can not be used to walk through numa node.
>>> Also, a valid node may be greater than nr_online_nodes.
>>>
>>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>>> all the places that use nr_online_nodes, and repair them one by one.
>>>
>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of
>>> gigantic pages can't work")
>>> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages
>>> parameter to support node allocation")
>>> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages
>>> parameter")
>>> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue
>>> warnings")
>>> Signed-off-by: Peng Liu <liupeng256@huawei.com>
>>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
>>
>> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>>
>> ... but
>>
>>> ---
>>> mm/hugetlb.c | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index b34f50156f7e..5b5a2a5a742f 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate
>>> *h, int nid)
>>> struct huge_bootmem_page *m = NULL; /* initialize for clang */
>>> int nr_nodes, node;
>>>
>>> - if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
>>> + if (nid != NUMA_NO_NODE && !node_online(nid))
>>
>> afaict null_blk could also use this, actually the whole thing wants a
>> helper - node_valid()?
>>
> This one should be unnecessary, and this patch looks has a bug,
>
> if a very nid passed to node_online(), it may crash, could you
> re-check it,
>
> see my changes below,
>
> 1) add tmp check against MAX_NUMNODES before node_online() check,
>
> and move it after get tmp in hugepages_setup() , this could cover
> both per-node alloc and normal alloc
sorry,for normal alloc, tmp is the number of huge pages, we don't need
the movement, only add tmp >= MAX_NUMNODES is ok
>
> 2) due to for_each_online_node() usage, we can drop additional check
> of nid in __alloc_bootmem_huge_page()
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-16 1:21 ` Kefeng Wang
@ 2022-04-19 4:40 ` Andrew Morton
2022-04-19 8:54 ` Kefeng Wang
0 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2022-04-19 4:40 UTC (permalink / raw)
To: Kefeng Wang
Cc: Peng Liu, mike.kravetz, david, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel
On Sat, 16 Apr 2022 09:21:45 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
> On 2022/4/15 13:41, Kefeng Wang wrote:
> >
> > On 2022/4/15 10:09, Davidlohr Bueso wrote:
> >> On Wed, 13 Apr 2022, Peng Liu wrote:
> >>
> >>> Certain systems are designed to have sparse/discontiguous nodes. In
> >>> this case, nr_online_nodes can not be used to walk through numa node.
> >>> Also, a valid node may be greater than nr_online_nodes.
> >>>
> >>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> >>> all the places that use nr_online_nodes, and repair them one by one.
> >>>
> >>> Suggested-by: David Hildenbrand <david@redhat.com>
> >>> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of
> >>> gigantic pages can't work")
> >>> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages
> >>> parameter to support node allocation")
> >>> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages
> >>> parameter")
> >>> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue
> >>> warnings")
> >>> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> >>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> >>> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> >>
> >> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> >>
> >> ... but
> >>
> >>> ---
> >>> mm/hugetlb.c | 12 ++++++------
> >>> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> >>> index b34f50156f7e..5b5a2a5a742f 100644
> >>> --- a/mm/hugetlb.c
> >>> +++ b/mm/hugetlb.c
> >>> @@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate
> >>> *h, int nid)
> >>> struct huge_bootmem_page *m = NULL; /* initialize for clang */
> >>> int nr_nodes, node;
> >>>
> >>> - if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
> >>> + if (nid != NUMA_NO_NODE && !node_online(nid))
> >>
> >> afaict null_blk could also use this, actually the whole thing wants a
> >> helper - node_valid()?
> >>
> > This one should be unnecessary, and this patch looks has a bug,
> >
> > if a very nid passed to node_online(), it may crash, could you
> > re-check it,
> >
> > see my changes below,
> >
> > 1) add tmp check against MAX_NUMNODES before node_online() check,
> >
> > and move it after get tmp in hugepages_setup() , this could cover
> > both per-node alloc and normal alloc
>
> sorry,for normal alloc, tmp is the number of huge pages, we don't need
> the movement, only add tmp >= MAX_NUMNODES is ok
>
Does the v4 patch address the issues which were raised in this thread?
--- a/mm/hugetlb.c~hugetlb-fix-wrong-use-of-nr_online_nodes-v4
+++ a/mm/hugetlb.c
@@ -2986,8 +2986,6 @@ int __alloc_bootmem_huge_page(struct hst
struct huge_bootmem_page *m = NULL; /* initialize for clang */
int nr_nodes, node;
- if (nid != NUMA_NO_NODE && !node_online(nid))
- return 0;
/* do node specific alloc */
if (nid != NUMA_NO_NODE) {
m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
@@ -4174,7 +4172,7 @@ static int __init hugepages_setup(char *
pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
return 0;
}
- if (!node_online(tmp))
+ if (tmp >= MAX_NUMNODES || !node_online(tmp))
goto invalid;
node = array_index_nospec(tmp, MAX_NUMNODES);
p += count + 1;
_
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-19 4:40 ` Andrew Morton
@ 2022-04-19 8:54 ` Kefeng Wang
0 siblings, 0 replies; 37+ messages in thread
From: Kefeng Wang @ 2022-04-19 8:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Peng Liu, mike.kravetz, david, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel
On 2022/4/19 12:40, Andrew Morton wrote:
> On Sat, 16 Apr 2022 09:21:45 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
>> On 2022/4/15 13:41, Kefeng Wang wrote:
>>> On 2022/4/15 10:09, Davidlohr Bueso wrote:
>>>> On Wed, 13 Apr 2022, Peng Liu wrote:
>>>>
>>>>> Certain systems are designed to have sparse/discontiguous nodes. In
>>>>> this case, nr_online_nodes can not be used to walk through numa node.
>>>>> Also, a valid node may be greater than nr_online_nodes.
>>>>>
>>>>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>>>>> all the places that use nr_online_nodes, and repair them one by one.
>>>>>
>>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>>> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of
>>>>> gigantic pages can't work")
>>>>> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages
>>>>> parameter to support node allocation")
>>>>> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages
>>>>> parameter")
>>>>> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue
>>>>> warnings")
>>>>> Signed-off-by: Peng Liu <liupeng256@huawei.com>
>>>>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>>> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
>>>> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>>>>
>>>> ... but
>>>>
>>>>> ---
>>>>> mm/hugetlb.c | 12 ++++++------
>>>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>>>> index b34f50156f7e..5b5a2a5a742f 100644
>>>>> --- a/mm/hugetlb.c
>>>>> +++ b/mm/hugetlb.c
>>>>> @@ -2979,7 +2979,7 @@ int __alloc_bootmem_huge_page(struct hstate
>>>>> *h, int nid)
>>>>> struct huge_bootmem_page *m = NULL; /* initialize for clang */
>>>>> int nr_nodes, node;
>>>>>
>>>>> - if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
>>>>> + if (nid != NUMA_NO_NODE && !node_online(nid))
>>>> afaict null_blk could also use this, actually the whole thing wants a
>>>> helper - node_valid()?
>>>>
>>> This one should be unnecessary, and this patch looks has a bug,
>>>
>>> if a very nid passed to node_online(), it may crash, could you
>>> re-check it,
>>>
>>> see my changes below,
>>>
>>> 1) add tmp check against MAX_NUMNODES before node_online() check,
>>>
>>> and move it after get tmp in hugepages_setup() , this could cover
>>> both per-node alloc and normal alloc
>> sorry,for normal alloc, tmp is the number of huge pages, we don't need
>> the movement, only add tmp >= MAX_NUMNODES is ok
>>
> Does the v4 patch address the issues which were raised in this thread?
Yes, v4 has fix this.
>
>
> --- a/mm/hugetlb.c~hugetlb-fix-wrong-use-of-nr_online_nodes-v4
> +++ a/mm/hugetlb.c
> @@ -2986,8 +2986,6 @@ int __alloc_bootmem_huge_page(struct hst
> struct huge_bootmem_page *m = NULL; /* initialize for clang */
> int nr_nodes, node;
>
> - if (nid != NUMA_NO_NODE && !node_online(nid))
> - return 0;
> /* do node specific alloc */
> if (nid != NUMA_NO_NODE) {
> m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
> @@ -4174,7 +4172,7 @@ static int __init hugepages_setup(char *
> pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
> return 0;
> }
> - if (!node_online(tmp))
> + if (tmp >= MAX_NUMNODES || !node_online(tmp))
> goto invalid;
> node = array_index_nospec(tmp, MAX_NUMNODES);
> p += count + 1;
> _
>
> .
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
` (3 preceding siblings ...)
2022-04-15 2:09 ` Davidlohr Bueso
@ 2022-04-16 10:35 ` Peng Liu
2022-04-18 5:53 ` Kefeng Wang
` (2 more replies)
4 siblings, 3 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-16 10:35 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, wangkefeng.wang, dave,
liupeng256
Certain systems are designed to have sparse/discontiguous nodes. In
this case, nr_online_nodes can not be used to walk through numa node.
Also, a valid node may be greater than nr_online_nodes.
However, in hugetlb, it is assumed that nodes are contiguous. Recheck
all the places that use nr_online_nodes, and repair them one by one.
Suggested-by: David Hildenbrand <david@redhat.com>
Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
v3->v4:
Make sure nid is valid before use node_online, and __alloc_bootmem_huge_page
is no need to check node_online, which is suggested by Kefeng.
mm/hugetlb.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b34f50156f7e..a386c5f94932 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2979,8 +2979,6 @@ int __alloc_bootmem_huge_page(struct hstate *h, int nid)
struct huge_bootmem_page *m = NULL; /* initialize for clang */
int nr_nodes, node;
- if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
- return 0;
/* do node specific alloc */
if (nid != NUMA_NO_NODE) {
m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
@@ -3088,7 +3086,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
}
/* do node specific alloc */
- for (i = 0; i < nr_online_nodes; i++) {
+ for_each_online_node(i) {
if (h->max_huge_pages_node[i] > 0) {
hugetlb_hstate_alloc_pages_onenode(h, i);
node_specific_alloc = true;
@@ -4049,7 +4047,7 @@ static int __init hugetlb_init(void)
default_hstate.max_huge_pages =
default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
}
@@ -4164,9 +4162,9 @@ static int __init hugepages_setup(char *s)
pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
return 0;
}
- if (tmp >= nr_online_nodes)
+ if (tmp >= MAX_NUMNODES || !node_online(tmp))
goto invalid;
- node = array_index_nospec(tmp, nr_online_nodes);
+ node = array_index_nospec(tmp, MAX_NUMNODES);
p += count + 1;
/* Parse hugepages */
if (sscanf(p, "%lu%n", &tmp, &count) != 1)
@@ -4294,7 +4292,7 @@ static int __init default_hugepagesz_setup(char *s)
*/
if (default_hstate_max_huge_pages) {
default_hstate.max_huge_pages = default_hstate_max_huge_pages;
- for (i = 0; i < nr_online_nodes; i++)
+ for_each_online_node(i)
default_hstate.max_huge_pages_node[i] =
default_hugepages_in_node[i];
if (hstate_is_gigantic(&default_hstate))
--
2.25.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-16 10:35 ` [PATCH v4] " Peng Liu
@ 2022-04-18 5:53 ` Kefeng Wang
2022-04-19 4:03 ` Andrew Morton
2022-04-29 9:32 ` David Hildenbrand
2 siblings, 0 replies; 37+ messages in thread
From: Kefeng Wang @ 2022-04-18 5:53 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel, dave
On 2022/4/16 18:35, Peng Liu wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> v3->v4:
> Make sure nid is valid before use node_online, and __alloc_bootmem_huge_page
> is no need to check node_online, which is suggested by Kefeng.
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> mm/hugetlb.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index b34f50156f7e..a386c5f94932 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2979,8 +2979,6 @@ int __alloc_bootmem_huge_page(struct hstate *h, int nid)
> struct huge_bootmem_page *m = NULL; /* initialize for clang */
> int nr_nodes, node;
>
> - if (nid != NUMA_NO_NODE && nid >= nr_online_nodes)
> - return 0;
> /* do node specific alloc */
> if (nid != NUMA_NO_NODE) {
> m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
> @@ -3088,7 +3086,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
> }
>
> /* do node specific alloc */
> - for (i = 0; i < nr_online_nodes; i++) {
> + for_each_online_node(i) {
> if (h->max_huge_pages_node[i] > 0) {
> hugetlb_hstate_alloc_pages_onenode(h, i);
> node_specific_alloc = true;
> @@ -4049,7 +4047,7 @@ static int __init hugetlb_init(void)
> default_hstate.max_huge_pages =
> default_hstate_max_huge_pages;
>
> - for (i = 0; i < nr_online_nodes; i++)
> + for_each_online_node(i)
> default_hstate.max_huge_pages_node[i] =
> default_hugepages_in_node[i];
> }
> @@ -4164,9 +4162,9 @@ static int __init hugepages_setup(char *s)
> pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
> return 0;
> }
> - if (tmp >= nr_online_nodes)
> + if (tmp >= MAX_NUMNODES || !node_online(tmp))
> goto invalid;
> - node = array_index_nospec(tmp, nr_online_nodes);
> + node = array_index_nospec(tmp, MAX_NUMNODES);
> p += count + 1;
> /* Parse hugepages */
> if (sscanf(p, "%lu%n", &tmp, &count) != 1)
> @@ -4294,7 +4292,7 @@ static int __init default_hugepagesz_setup(char *s)
> */
> if (default_hstate_max_huge_pages) {
> default_hstate.max_huge_pages = default_hstate_max_huge_pages;
> - for (i = 0; i < nr_online_nodes; i++)
> + for_each_online_node(i)
> default_hstate.max_huge_pages_node[i] =
> default_hugepages_in_node[i];
> if (hstate_is_gigantic(&default_hstate))
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-16 10:35 ` [PATCH v4] " Peng Liu
2022-04-18 5:53 ` Kefeng Wang
@ 2022-04-19 4:03 ` Andrew Morton
2022-04-19 14:07 ` Kefeng Wang
2022-04-29 9:32 ` David Hildenbrand
2 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2022-04-19 4:03 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, wangkefeng.wang, dave
On Sat, 16 Apr 2022 10:35:26 +0000 Peng Liu <liupeng256@huawei.com> wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
oops.
What are the user-visible runtime effects of this flaw?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-19 4:03 ` Andrew Morton
@ 2022-04-19 14:07 ` Kefeng Wang
2022-04-20 6:17 ` liupeng (DM)
0 siblings, 1 reply; 37+ messages in thread
From: Kefeng Wang @ 2022-04-19 14:07 UTC (permalink / raw)
To: Andrew Morton, Peng Liu
Cc: mike.kravetz, david, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, dave
On 2022/4/19 12:03, Andrew Morton wrote:
> On Sat, 16 Apr 2022 10:35:26 +0000 Peng Liu <liupeng256@huawei.com> wrote:
>
>> Certain systems are designed to have sparse/discontiguous nodes. In
>> this case, nr_online_nodes can not be used to walk through numa node.
>> Also, a valid node may be greater than nr_online_nodes.
>>
>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>> all the places that use nr_online_nodes, and repair them one by one.
> oops.
>
> What are the user-visible runtime effects of this flaw?
For example, there are four node=0,1,2,3, and nid = 1 is offline
node,nr_online_nodes = 3
1) per-node alloc (hugepages=1:2) fails,
2) per-node alloc (hugepages=3:2) fails, but it could succeed.
I assume that there is no user-visible runtime effects.
> .
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-19 14:07 ` Kefeng Wang
@ 2022-04-20 6:17 ` liupeng (DM)
0 siblings, 0 replies; 37+ messages in thread
From: liupeng (DM) @ 2022-04-20 6:17 UTC (permalink / raw)
To: Kefeng Wang, Andrew Morton
Cc: mike.kravetz, david, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, dave
On 2022/4/19 22:07, Kefeng Wang wrote:
>
> On 2022/4/19 12:03, Andrew Morton wrote:
>> On Sat, 16 Apr 2022 10:35:26 +0000 Peng Liu <liupeng256@huawei.com>
>> wrote:
>>
>>> Certain systems are designed to have sparse/discontiguous nodes. In
>>> this case, nr_online_nodes can not be used to walk through numa node.
>>> Also, a valid node may be greater than nr_online_nodes.
>>>
>>> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
>>> all the places that use nr_online_nodes, and repair them one by one.
>> oops.
>>
>> What are the user-visible runtime effects of this flaw?
>
> For example, there are four node=0,1,2,3, and nid = 1 is offline
> node,nr_online_nodes = 3
>
> 1) per-node alloc (hugepages=1:2) fails,
>
> 2) per-node alloc (hugepages=3:2) fails, but it could succeed.
>
> I assume that there is no user-visible runtime effects.
>
Thanks, you are right.
I have constructed node =0, 1, 3, 4, and requested huge pages as:
hugepagesz=1G hugepages=0:1,4:1 hugepagesz=2M hugepages=0:1024,4:1024
Without this patch:
HugeTLB: Invalid hugepages parameter 4:1
HugeTLB: Invalid hugepages parameter 4:1024
HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
HugeTLB registered 2.00 MiB page size, pre-allocated 1024 pages
With this patch:
HugeTLB registered 1.00 GiB page size, pre-allocated 2 pages
HugeTLB registered 2.00 MiB page size, pre-allocated 2048 pages
>> .
> .
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] hugetlb: Fix wrong use of nr_online_nodes
2022-04-16 10:35 ` [PATCH v4] " Peng Liu
2022-04-18 5:53 ` Kefeng Wang
2022-04-19 4:03 ` Andrew Morton
@ 2022-04-29 9:32 ` David Hildenbrand
2 siblings, 0 replies; 37+ messages in thread
From: David Hildenbrand @ 2022-04-29 9:32 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel, wangkefeng.wang,
dave
On 16.04.22 12:35, Peng Liu wrote:
> Certain systems are designed to have sparse/discontiguous nodes. In
> this case, nr_online_nodes can not be used to walk through numa node.
> Also, a valid node may be greater than nr_online_nodes.
>
> However, in hugetlb, it is assumed that nodes are contiguous. Recheck
> all the places that use nr_online_nodes, and repair them one by one.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 4178158ef8ca ("hugetlbfs: fix issue of preallocation of gigantic pages can't work")
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Fixes: e79ce9832316 ("hugetlbfs: fix a truncation issue in hugepages parameter")
> Fixes: f9317f77a6e0 ("hugetlb: clean up potential spectre issue warnings")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode
2022-04-13 3:29 [PATCH v3 0/4] hugetlb: Fix some incorrect behavior Peng Liu
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
@ 2022-04-13 3:29 ` Peng Liu
2022-04-14 23:50 ` Mike Kravetz
2022-04-29 9:30 ` David Hildenbrand
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
3 siblings, 2 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-13 3:29 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, liupeng256
Hugepages can be specified to pernode since "hugetlbfs: extend
the definition of hugepages parameter to support node allocation",
but the following problem is observed.
Confusing behavior is observed when both 1G and 2M hugepage is set
after "numa=off".
cmdline hugepage settings:
hugepagesz=1G hugepages=0:3,1:3
hugepagesz=2M hugepages=0:1024,1:1024
results:
HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
HugeTLB registered 2.00 MiB page size, pre-allocated 1024 pages
Furthermore, confusing behavior can be also observed when an
invalid node behind a valid node. To fix this, never allocate any
typical hugepage when an invalid parameter is received.
Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
mm/hugetlb.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 5b5a2a5a742f..1930b6341f7e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4116,6 +4116,20 @@ bool __init __weak hugetlb_node_alloc_supported(void)
{
return true;
}
+
+static void __init hugepages_clear_pages_in_node(void)
+{
+ if (!hugetlb_max_hstate) {
+ default_hstate_max_huge_pages = 0;
+ memset(default_hugepages_in_node, 0,
+ MAX_NUMNODES * sizeof(unsigned int));
+ } else {
+ parsed_hstate->max_huge_pages = 0;
+ memset(parsed_hstate->max_huge_pages_node, 0,
+ MAX_NUMNODES * sizeof(unsigned int));
+ }
+}
+
/*
* hugepages command line processing
* hugepages normally follows a valid hugepagsz or default_hugepagsz
@@ -4203,6 +4217,7 @@ static int __init hugepages_setup(char *s)
invalid:
pr_warn("HugeTLB: Invalid hugepages parameter %s\n", p);
+ hugepages_clear_pages_in_node();
return 0;
}
__setup("hugepages=", hugepages_setup);
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode
2022-04-13 3:29 ` [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode Peng Liu
@ 2022-04-14 23:50 ` Mike Kravetz
2022-04-29 9:30 ` David Hildenbrand
1 sibling, 0 replies; 37+ messages in thread
From: Mike Kravetz @ 2022-04-14 23:50 UTC (permalink / raw)
To: Peng Liu, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/12/22 20:29, Peng Liu wrote:
> Hugepages can be specified to pernode since "hugetlbfs: extend
> the definition of hugepages parameter to support node allocation",
> but the following problem is observed.
>
> Confusing behavior is observed when both 1G and 2M hugepage is set
> after "numa=off".
> cmdline hugepage settings:
> hugepagesz=1G hugepages=0:3,1:3
> hugepagesz=2M hugepages=0:1024,1:1024
> results:
> HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
> HugeTLB registered 2.00 MiB page size, pre-allocated 1024 pages
>
> Furthermore, confusing behavior can be also observed when an
> invalid node behind a valid node. To fix this, never allocate any
> typical hugepage when an invalid parameter is received.
>
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Thanks!
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode
2022-04-13 3:29 ` [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode Peng Liu
2022-04-14 23:50 ` Mike Kravetz
@ 2022-04-29 9:30 ` David Hildenbrand
1 sibling, 0 replies; 37+ messages in thread
From: David Hildenbrand @ 2022-04-29 9:30 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel
On 13.04.22 05:29, Peng Liu wrote:
> Hugepages can be specified to pernode since "hugetlbfs: extend
> the definition of hugepages parameter to support node allocation",
> but the following problem is observed.
>
> Confusing behavior is observed when both 1G and 2M hugepage is set
> after "numa=off".
> cmdline hugepage settings:
> hugepagesz=1G hugepages=0:3,1:3
> hugepagesz=2M hugepages=0:1024,1:1024
> results:
> HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
> HugeTLB registered 2.00 MiB page size, pre-allocated 1024 pages
>
> Furthermore, confusing behavior can be also observed when an
> invalid node behind a valid node. To fix this, never allocate any
> typical hugepage when an invalid parameter is received.
>
> Fixes: b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 5b5a2a5a742f..1930b6341f7e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4116,6 +4116,20 @@ bool __init __weak hugetlb_node_alloc_supported(void)
> {
> return true;
> }
> +
> +static void __init hugepages_clear_pages_in_node(void)
I think the name is a bit imprecise, but I have no better suggestion
right now.
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers
2022-04-13 3:29 [PATCH v3 0/4] hugetlb: Fix some incorrect behavior Peng Liu
2022-04-13 3:29 ` [PATCH v3 1/4] hugetlb: Fix wrong use of nr_online_nodes Peng Liu
2022-04-13 3:29 ` [PATCH v3 2/4] hugetlb: Fix hugepages_setup when deal with pernode Peng Liu
@ 2022-04-13 3:29 ` Peng Liu
2022-04-13 6:39 ` Baolin Wang
` (4 more replies)
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
3 siblings, 5 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-13 3:29 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, liupeng256
When __setup() return '0', using invalid option values causes the
entire kernel boot option string to be reported as Unknown. Hugetlb
calls __setup() and will return '0' when set invalid parameter
string.
The following phenomenon is observed:
cmdline:
hugepagesz=1Y hugepages=1
dmesg:
HugeTLB: unsupported hugepagesz=1Y
HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
Unknown kernel command line parameters "hugepagesz=1Y hugepages=1"
Since hugetlb will print warning/error information before return for
invalid parameter string, just use return '1' to avoid print again.
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
mm/hugetlb.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 1930b6341f7e..2e4d8d9fb7c6 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4149,7 +4149,7 @@ static int __init hugepages_setup(char *s)
if (!parsed_valid_hugepagesz) {
pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s);
parsed_valid_hugepagesz = true;
- return 0;
+ return 1;
}
/*
@@ -4165,7 +4165,7 @@ static int __init hugepages_setup(char *s)
if (mhp == last_mhp) {
pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n", s);
- return 0;
+ return 1;
}
while (*p) {
@@ -4176,7 +4176,7 @@ static int __init hugepages_setup(char *s)
if (p[count] == ':') {
if (!hugetlb_node_alloc_supported()) {
pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
- return 0;
+ return 1;
}
if (!node_online(tmp))
goto invalid;
@@ -4218,7 +4218,7 @@ static int __init hugepages_setup(char *s)
invalid:
pr_warn("HugeTLB: Invalid hugepages parameter %s\n", p);
hugepages_clear_pages_in_node();
- return 0;
+ return 1;
}
__setup("hugepages=", hugepages_setup);
@@ -4239,7 +4239,7 @@ static int __init hugepagesz_setup(char *s)
if (!arch_hugetlb_valid_size(size)) {
pr_err("HugeTLB: unsupported hugepagesz=%s\n", s);
- return 0;
+ return 1;
}
h = size_to_hstate(size);
@@ -4254,7 +4254,7 @@ static int __init hugepagesz_setup(char *s)
if (!parsed_default_hugepagesz || h != &default_hstate ||
default_hstate.max_huge_pages) {
pr_warn("HugeTLB: hugepagesz=%s specified twice, ignoring\n", s);
- return 0;
+ return 1;
}
/*
@@ -4285,14 +4285,14 @@ static int __init default_hugepagesz_setup(char *s)
parsed_valid_hugepagesz = false;
if (parsed_default_hugepagesz) {
pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s);
- return 0;
+ return 1;
}
size = (unsigned long)memparse(s, NULL);
if (!arch_hugetlb_valid_size(size)) {
pr_err("HugeTLB: unsupported default_hugepagesz=%s\n", s);
- return 0;
+ return 1;
}
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
@ 2022-04-13 6:39 ` Baolin Wang
2022-04-13 7:55 ` Muchun Song
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Baolin Wang @ 2022-04-13 6:39 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/13/2022 11:29 AM, Peng Liu wrote:
> When __setup() return '0', using invalid option values causes the
> entire kernel boot option string to be reported as Unknown. Hugetlb
> calls __setup() and will return '0' when set invalid parameter
> string.
>
> The following phenomenon is observed:
> cmdline:
> hugepagesz=1Y hugepages=1
> dmesg:
> HugeTLB: unsupported hugepagesz=1Y
> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1"
>
> Since hugetlb will print warning/error information before return for
> invalid parameter string, just use return '1' to avoid print again.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
2022-04-13 6:39 ` Baolin Wang
@ 2022-04-13 7:55 ` Muchun Song
[not found] ` <5bbf45e7-1d92-f543-5cfc-bc0141999d46@huawei.com>
2022-04-15 0:01 ` Mike Kravetz
` (2 subsequent siblings)
4 siblings, 1 reply; 37+ messages in thread
From: Muchun Song @ 2022-04-13 7:55 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, liuyuntao10,
linux-mm, linux-kernel
On Wed, Apr 13, 2022 at 03:29:14AM +0000, Peng Liu wrote:
> When __setup() return '0', using invalid option values causes the
> entire kernel boot option string to be reported as Unknown. Hugetlb
> calls __setup() and will return '0' when set invalid parameter
> string.
>
> The following phenomenon is observed:
> cmdline:
> hugepagesz=1Y hugepages=1
> dmesg:
> HugeTLB: unsupported hugepagesz=1Y
> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1"
>
> Since hugetlb will print warning/error information before return for
> invalid parameter string, just use return '1' to avoid print again.
>
Can't return -EINVAL? It is weird to return 1 on failure.
Thanks.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
2022-04-13 6:39 ` Baolin Wang
2022-04-13 7:55 ` Muchun Song
@ 2022-04-15 0:01 ` Mike Kravetz
2022-04-15 2:24 ` Davidlohr Bueso
2022-04-29 3:02 ` [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding Peng Liu
4 siblings, 0 replies; 37+ messages in thread
From: Mike Kravetz @ 2022-04-15 0:01 UTC (permalink / raw)
To: Peng Liu, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/12/22 20:29, Peng Liu wrote:
> When __setup() return '0', using invalid option values causes the
> entire kernel boot option string to be reported as Unknown. Hugetlb
> calls __setup() and will return '0' when set invalid parameter
> string.
>
> The following phenomenon is observed:
> cmdline:
> hugepagesz=1Y hugepages=1
> dmesg:
> HugeTLB: unsupported hugepagesz=1Y
> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1"
>
> Since hugetlb will print warning/error information before return for
> invalid parameter string, just use return '1' to avoid print again.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
Thank you for cleaning this up!
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
` (2 preceding siblings ...)
2022-04-15 0:01 ` Mike Kravetz
@ 2022-04-15 2:24 ` Davidlohr Bueso
2022-04-29 3:02 ` [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding Peng Liu
4 siblings, 0 replies; 37+ messages in thread
From: Davidlohr Bueso @ 2022-04-15 2:24 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On Wed, 13 Apr 2022, Peng Liu wrote:
>When __setup() return '0', using invalid option values causes the
>entire kernel boot option string to be reported as Unknown. Hugetlb
>calls __setup() and will return '0' when set invalid parameter
>string.
>
>The following phenomenon is observed:
> cmdline:
> hugepagesz=1Y hugepages=1
> dmesg:
> HugeTLB: unsupported hugepagesz=1Y
> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1"
>
>Since hugetlb will print warning/error information before return for
>invalid parameter string, just use return '1' to avoid print again.
>
>Signed-off-by: Peng Liu <liupeng256@huawei.com>
>Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>Reviewed-by: Muchun Song <songmuchun@bytedance.com>
>Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
` (3 preceding siblings ...)
2022-04-15 2:24 ` Davidlohr Bueso
@ 2022-04-29 3:02 ` Peng Liu
2022-04-29 9:29 ` David Hildenbrand
2022-04-29 11:44 ` Muchun Song
4 siblings, 2 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-29 3:02 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, wangkefeng.wang, dave,
liupeng256, wangborong, linux-ia64, adobriyan
Use more generic functions to deal with issues related to online
nodes. The changes will make the code simplified.
Signed-off-by: Peng Liu <liupeng256@huawei.com>
Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
---
v4:
Clean up all the related issues in one patch as suggested by Andrew.
arch/ia64/kernel/uncached.c | 2 +-
mm/hugetlb.c | 4 ++--
mm/page_ext.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/ia64/kernel/uncached.c b/arch/ia64/kernel/uncached.c
index 816803636a75..a0fec82c56b8 100644
--- a/arch/ia64/kernel/uncached.c
+++ b/arch/ia64/kernel/uncached.c
@@ -261,7 +261,7 @@ static int __init uncached_init(void)
{
int nid;
- for_each_node_state(nid, N_ONLINE) {
+ for_each_online_node(nid) {
uncached_pools[nid].pool = gen_pool_create(PAGE_SHIFT, nid);
mutex_init(&uncached_pools[nid].add_chunk_mutex);
}
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 30e1099fd99a..0e5a7764efaa 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6951,7 +6951,7 @@ void __init hugetlb_cma_reserve(int order)
if (hugetlb_cma_size_in_node[nid] == 0)
continue;
- if (!node_state(nid, N_ONLINE)) {
+ if (!node_online(nid)) {
pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
hugetlb_cma_size_in_node[nid] = 0;
@@ -6990,7 +6990,7 @@ void __init hugetlb_cma_reserve(int order)
}
reserved = 0;
- for_each_node_state(nid, N_ONLINE) {
+ for_each_online_node(nid) {
int res;
char name[CMA_MAX_NAME];
diff --git a/mm/page_ext.c b/mm/page_ext.c
index 2e66d934d63f..3dc715d7ac29 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -320,7 +320,7 @@ static int __meminit online_page_ext(unsigned long start_pfn,
* online__pages(), and start_pfn should exist.
*/
nid = pfn_to_nid(start_pfn);
- VM_BUG_ON(!node_state(nid, N_ONLINE));
+ VM_BUG_ON(!node_online(nid));
}
for (pfn = start; !fail && pfn < end; pfn += PAGES_PER_SECTION)
--
2.25.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding
2022-04-29 3:02 ` [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding Peng Liu
@ 2022-04-29 9:29 ` David Hildenbrand
2022-04-29 11:44 ` Muchun Song
1 sibling, 0 replies; 37+ messages in thread
From: David Hildenbrand @ 2022-04-29 9:29 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel, wangkefeng.wang,
dave, wangborong, linux-ia64, adobriyan
On 29.04.22 05:02, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> ---
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding
2022-04-29 3:02 ` [PATCH v4] mm: Using for_each_online_node and node_online instead of open coding Peng Liu
2022-04-29 9:29 ` David Hildenbrand
@ 2022-04-29 11:44 ` Muchun Song
1 sibling, 0 replies; 37+ messages in thread
From: Muchun Song @ 2022-04-29 11:44 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, liuyuntao10,
linux-mm, linux-kernel, wangkefeng.wang, dave, wangborong,
linux-ia64, adobriyan
On Fri, Apr 29, 2022 at 03:02:18AM +0000, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Thanks.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 [PATCH v3 0/4] hugetlb: Fix some incorrect behavior Peng Liu
` (2 preceding siblings ...)
2022-04-13 3:29 ` [PATCH v3 3/4] hugetlb: Fix return value of __setup handlers Peng Liu
@ 2022-04-13 3:29 ` Peng Liu
2022-04-13 5:50 ` Muchun Song
` (4 more replies)
3 siblings, 5 replies; 37+ messages in thread
From: Peng Liu @ 2022-04-13 3:29 UTC (permalink / raw)
To: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel, liupeng256
Use more generic functions to deal with issues related to online
nodes. The changes will make the code simplified.
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
mm/hugetlb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2e4d8d9fb7c6..4c529774cc08 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6953,7 +6953,7 @@ void __init hugetlb_cma_reserve(int order)
if (hugetlb_cma_size_in_node[nid] == 0)
continue;
- if (!node_state(nid, N_ONLINE)) {
+ if (!node_online(nid)) {
pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
hugetlb_cma_size_in_node[nid] = 0;
@@ -6992,7 +6992,7 @@ void __init hugetlb_cma_reserve(int order)
}
reserved = 0;
- for_each_node_state(nid, N_ONLINE) {
+ for_each_online_node(nid) {
int res;
char name[CMA_MAX_NAME];
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
@ 2022-04-13 5:50 ` Muchun Song
2022-04-13 6:41 ` Baolin Wang
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Muchun Song @ 2022-04-13 5:50 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, liuyuntao10,
linux-mm, linux-kernel
On Wed, Apr 13, 2022 at 03:29:15AM +0000, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
LGTM.
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Thanks.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
2022-04-13 5:50 ` Muchun Song
@ 2022-04-13 6:41 ` Baolin Wang
2022-04-15 0:03 ` Mike Kravetz
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Baolin Wang @ 2022-04-13 6:41 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, david, akpm, yaozhenguo1, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/13/2022 11:29 AM, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
Looks more consistent. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> mm/hugetlb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 2e4d8d9fb7c6..4c529774cc08 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6953,7 +6953,7 @@ void __init hugetlb_cma_reserve(int order)
> if (hugetlb_cma_size_in_node[nid] == 0)
> continue;
>
> - if (!node_state(nid, N_ONLINE)) {
> + if (!node_online(nid)) {
> pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
> hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
> hugetlb_cma_size_in_node[nid] = 0;
> @@ -6992,7 +6992,7 @@ void __init hugetlb_cma_reserve(int order)
> }
>
> reserved = 0;
> - for_each_node_state(nid, N_ONLINE) {
> + for_each_online_node(nid) {
> int res;
> char name[CMA_MAX_NAME];
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
2022-04-13 5:50 ` Muchun Song
2022-04-13 6:41 ` Baolin Wang
@ 2022-04-15 0:03 ` Mike Kravetz
2022-04-15 2:15 ` Davidlohr Bueso
2022-04-29 9:28 ` David Hildenbrand
4 siblings, 0 replies; 37+ messages in thread
From: Mike Kravetz @ 2022-04-15 0:03 UTC (permalink / raw)
To: Peng Liu, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On 4/12/22 20:29, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thank you!
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
` (2 preceding siblings ...)
2022-04-15 0:03 ` Mike Kravetz
@ 2022-04-15 2:15 ` Davidlohr Bueso
2022-04-29 9:28 ` David Hildenbrand
4 siblings, 0 replies; 37+ messages in thread
From: Davidlohr Bueso @ 2022-04-15 2:15 UTC (permalink / raw)
To: Peng Liu
Cc: mike.kravetz, david, akpm, yaozhenguo1, baolin.wang, songmuchun,
liuyuntao10, linux-mm, linux-kernel
On Wed, 13 Apr 2022, Peng Liu wrote:
>Use more generic functions to deal with issues related to online
>nodes. The changes will make the code simplified.
>
>Signed-off-by: Peng Liu <liupeng256@huawei.com>
>Reviewed-by: Muchun Song <songmuchun@bytedance.com>
>Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>---
> mm/hugetlb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>index 2e4d8d9fb7c6..4c529774cc08 100644
>--- a/mm/hugetlb.c
>+++ b/mm/hugetlb.c
>@@ -6953,7 +6953,7 @@ void __init hugetlb_cma_reserve(int order)
> if (hugetlb_cma_size_in_node[nid] == 0)
> continue;
>
>- if (!node_state(nid, N_ONLINE)) {
>+ if (!node_online(nid)) {
You could update mm/page_ext.c as well
> pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
> hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
> hugetlb_cma_size_in_node[nid] = 0;
>@@ -6992,7 +6992,7 @@ void __init hugetlb_cma_reserve(int order)
> }
>
> reserved = 0;
>- for_each_node_state(nid, N_ONLINE) {
>+ for_each_online_node(nid) {
... and arch/ia64/kernel/uncached.c for this.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve
2022-04-13 3:29 ` [PATCH v3 4/4] hugetlb: Clean up hugetlb_cma_reserve Peng Liu
` (3 preceding siblings ...)
2022-04-15 2:15 ` Davidlohr Bueso
@ 2022-04-29 9:28 ` David Hildenbrand
4 siblings, 0 replies; 37+ messages in thread
From: David Hildenbrand @ 2022-04-29 9:28 UTC (permalink / raw)
To: Peng Liu, mike.kravetz, akpm, yaozhenguo1, baolin.wang,
songmuchun, liuyuntao10, linux-mm, linux-kernel
On 13.04.22 05:29, Peng Liu wrote:
> Use more generic functions to deal with issues related to online
> nodes. The changes will make the code simplified.
>
> Signed-off-by: Peng Liu <liupeng256@huawei.com>
> ---
> mm/hugetlb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 2e4d8d9fb7c6..4c529774cc08 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6953,7 +6953,7 @@ void __init hugetlb_cma_reserve(int order)
> if (hugetlb_cma_size_in_node[nid] == 0)
> continue;
>
> - if (!node_state(nid, N_ONLINE)) {
> + if (!node_online(nid)) {
> pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
> hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
> hugetlb_cma_size_in_node[nid] = 0;
> @@ -6992,7 +6992,7 @@ void __init hugetlb_cma_reserve(int order)
> }
>
> reserved = 0;
> - for_each_node_state(nid, N_ONLINE) {
> + for_each_online_node(nid) {
> int res;
> char name[CMA_MAX_NAME];
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 37+ messages in thread