| Message ID | 20200714073404.84863-1-richard.weiyang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/hugetlb: hide nr_nodes in the internal of for_each_node_mask_to_[alloc|free] | expand |
> The second parameter of for_each_node_mask_to_[alloc|free] is a loop > variant, which is not used outside of loop iteration. > > Let's hide this. > > Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> > --- > mm/hugetlb.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 57ece74e3aae..9c3d15fb317e 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) > return nid; > } > > -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ > - for (nr_nodes = nodes_weight(*mask); \ > - nr_nodes > 0 && \ > +#define for_each_node_mask_to_alloc(hs, node, mask) \ > + int __nr_nodes; \ > + for (__nr_nodes = nodes_weight(*mask); \ > + __nr_nodes > 0 && \ > ((node = hstate_next_node_to_alloc(hs, mask)) || 1); \ > - nr_nodes--) > + __nr_nodes--) > > -#define for_each_node_mask_to_free(hs, nr_nodes, node, mask) \ > - for (nr_nodes = nodes_weight(*mask); \ > - nr_nodes > 0 && \ > +#define for_each_node_mask_to_free(hs, node, mask) \ > + int __nr_nodes; \ > + for (__nr_nodes = nodes_weight(*mask); \ > + __nr_nodes > 0 && \ > ((node = hstate_next_node_to_free(hs, mask)) || 1); \ > - nr_nodes--) > + __nr_nodes--) > > #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE > static void destroy_compound_gigantic_page(struct page *page, > @@ -1403,7 +1405,7 @@ static void __free_huge_page(struct page *page) > * reservation. If the page was associated with a subpool, there > * would have been a page reserved in the subpool before allocation > * via hugepage_subpool_get_pages(). Since we are 'restoring' the > - * reservtion, do not call hugepage_subpool_put_pages() as this will > + * reservation, do not call hugepage_subpool_put_pages() as this will > * remove the reserved page from the subpool. > */ > if (!restore_reserve) { > @@ -1760,10 +1762,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > nodemask_t *node_alloc_noretry) > { > struct page *page; > - int nr_nodes, node; > + int node; > gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE; > > - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_alloc(h, node, nodes_allowed) { > page = alloc_fresh_huge_page(h, gfp_mask, node, nodes_allowed, > node_alloc_noretry); > if (page) > @@ -1787,10 +1789,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > bool acct_surplus) > { > - int nr_nodes, node; > + int node; > int ret = 0; > > - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_free(h, node, nodes_allowed) { > /* > * If we're returning unused surplus pages, only examine > * nodes with surplus pages. > @@ -2481,9 +2483,9 @@ int alloc_bootmem_huge_page(struct hstate *h) > int __alloc_bootmem_huge_page(struct hstate *h) > { > struct huge_bootmem_page *m; > - int nr_nodes, node; > + int node; > > - for_each_node_mask_to_alloc(h, nr_nodes, node, &node_states[N_MEMORY]) { > + for_each_node_mask_to_alloc(h, node, &node_states[N_MEMORY]) { > void *addr; > > addr = memblock_alloc_try_nid_raw( > @@ -2662,17 +2664,17 @@ static inline void try_to_free_low(struct hstate *h, unsigned long count, > static int adjust_pool_surplus(struct hstate *h, nodemask_t *nodes_allowed, > int delta) > { > - int nr_nodes, node; > + int node; > > VM_BUG_ON(delta != -1 && delta != 1); > > if (delta < 0) { > - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_alloc(h, node, nodes_allowed) { > if (h->surplus_huge_pages_node[node]) > goto found; > } > } else { > - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_free(h, node, nodes_allowed) { > if (h->surplus_huge_pages_node[node] < > h->nr_huge_pages_node[node]) > goto found; > -- > 2.20.1 (Apple Git-117) Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> > >
On 7/14/20 9:34 AM, Wei Yang wrote: > The second parameter of for_each_node_mask_to_[alloc|free] is a loop > variant, which is not used outside of loop iteration. > > Let's hide this. > > Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> > --- > mm/hugetlb.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 57ece74e3aae..9c3d15fb317e 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) > return nid; > } > > -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ > - for (nr_nodes = nodes_weight(*mask); \ > - nr_nodes > 0 && \ > +#define for_each_node_mask_to_alloc(hs, node, mask) \ > + int __nr_nodes; \ > + for (__nr_nodes = nodes_weight(*mask); \ The problem with this is that if I use the macro twice in the same block, this will redefine __nr_nodes and fail to compile, no? In that case it's better to avoid setting up this trap, IMHO. > + __nr_nodes > 0 && \ > ((node = hstate_next_node_to_alloc(hs, mask)) || 1); \ > - nr_nodes--) > + __nr_nodes--) > > -#define for_each_node_mask_to_free(hs, nr_nodes, node, mask) \ > - for (nr_nodes = nodes_weight(*mask); \ > - nr_nodes > 0 && \ > +#define for_each_node_mask_to_free(hs, node, mask) \ > + int __nr_nodes; \ > + for (__nr_nodes = nodes_weight(*mask); \ > + __nr_nodes > 0 && \ > ((node = hstate_next_node_to_free(hs, mask)) || 1); \ > - nr_nodes--) > + __nr_nodes--) > > #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE > static void destroy_compound_gigantic_page(struct page *page, > @@ -1403,7 +1405,7 @@ static void __free_huge_page(struct page *page) > * reservation. If the page was associated with a subpool, there > * would have been a page reserved in the subpool before allocation > * via hugepage_subpool_get_pages(). Since we are 'restoring' the > - * reservtion, do not call hugepage_subpool_put_pages() as this will > + * reservation, do not call hugepage_subpool_put_pages() as this will > * remove the reserved page from the subpool. > */ > if (!restore_reserve) { > @@ -1760,10 +1762,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > nodemask_t *node_alloc_noretry) > { > struct page *page; > - int nr_nodes, node; > + int node; > gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE; > > - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_alloc(h, node, nodes_allowed) { > page = alloc_fresh_huge_page(h, gfp_mask, node, nodes_allowed, > node_alloc_noretry); > if (page) > @@ -1787,10 +1789,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > bool acct_surplus) > { > - int nr_nodes, node; > + int node; > int ret = 0; > > - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_free(h, node, nodes_allowed) { > /* > * If we're returning unused surplus pages, only examine > * nodes with surplus pages. > @@ -2481,9 +2483,9 @@ int alloc_bootmem_huge_page(struct hstate *h) > int __alloc_bootmem_huge_page(struct hstate *h) > { > struct huge_bootmem_page *m; > - int nr_nodes, node; > + int node; > > - for_each_node_mask_to_alloc(h, nr_nodes, node, &node_states[N_MEMORY]) { > + for_each_node_mask_to_alloc(h, node, &node_states[N_MEMORY]) { > void *addr; > > addr = memblock_alloc_try_nid_raw( > @@ -2662,17 +2664,17 @@ static inline void try_to_free_low(struct hstate *h, unsigned long count, > static int adjust_pool_surplus(struct hstate *h, nodemask_t *nodes_allowed, > int delta) > { > - int nr_nodes, node; > + int node; > > VM_BUG_ON(delta != -1 && delta != 1); > > if (delta < 0) { > - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_alloc(h, node, nodes_allowed) { > if (h->surplus_huge_pages_node[node]) > goto found; > } > } else { > - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { > + for_each_node_mask_to_free(h, node, nodes_allowed) { > if (h->surplus_huge_pages_node[node] < > h->nr_huge_pages_node[node]) > goto found; >
On 7/14/20 11:13 AM, Vlastimil Babka wrote: > On 7/14/20 9:34 AM, Wei Yang wrote: >> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >> variant, which is not used outside of loop iteration. >> >> Let's hide this. >> >> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >> --- >> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >> 1 file changed, 20 insertions(+), 18 deletions(-) >> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 57ece74e3aae..9c3d15fb317e 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >> return nid; >> } >> >> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >> - for (nr_nodes = nodes_weight(*mask); \ >> - nr_nodes > 0 && \ >> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >> + int __nr_nodes; \ >> + for (__nr_nodes = nodes_weight(*mask); \ > > The problem with this is that if I use the macro twice in the same block, this > will redefine __nr_nodes and fail to compile, no? > In that case it's better to avoid setting up this trap, IMHO. Ah, and it will also generate the following warning, if the use of for_each* macro is not the first thing after variable declarations, but there's another statement before: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P
On Tue, Jul 14, 2020 at 11:22:03AM +0200, Vlastimil Babka wrote: >On 7/14/20 11:13 AM, Vlastimil Babka wrote: >> On 7/14/20 9:34 AM, Wei Yang wrote: >>> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >>> variant, which is not used outside of loop iteration. >>> >>> Let's hide this. >>> >>> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>> --- >>> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >>> 1 file changed, 20 insertions(+), 18 deletions(-) >>> >>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>> index 57ece74e3aae..9c3d15fb317e 100644 >>> --- a/mm/hugetlb.c >>> +++ b/mm/hugetlb.c >>> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >>> return nid; >>> } >>> >>> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >>> - for (nr_nodes = nodes_weight(*mask); \ >>> - nr_nodes > 0 && \ >>> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >>> + int __nr_nodes; \ >>> + for (__nr_nodes = nodes_weight(*mask); \ >> >> The problem with this is that if I use the macro twice in the same block, this >> will redefine __nr_nodes and fail to compile, no? >> In that case it's better to avoid setting up this trap, IMHO. > >Ah, and it will also generate the following warning, if the use of for_each* >macro is not the first thing after variable declarations, but there's another >statement before: > >warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] > >Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P Thanks, I haven't notice the problem you mentioned. Let me fix this.
On Tue, Jul 14, 2020 at 11:22:03AM +0200, Vlastimil Babka wrote: >On 7/14/20 11:13 AM, Vlastimil Babka wrote: >> On 7/14/20 9:34 AM, Wei Yang wrote: >>> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >>> variant, which is not used outside of loop iteration. >>> >>> Let's hide this. >>> >>> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>> --- >>> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >>> 1 file changed, 20 insertions(+), 18 deletions(-) >>> >>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>> index 57ece74e3aae..9c3d15fb317e 100644 >>> --- a/mm/hugetlb.c >>> +++ b/mm/hugetlb.c >>> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >>> return nid; >>> } >>> >>> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >>> - for (nr_nodes = nodes_weight(*mask); \ >>> - nr_nodes > 0 && \ >>> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >>> + int __nr_nodes; \ >>> + for (__nr_nodes = nodes_weight(*mask); \ >> >> The problem with this is that if I use the macro twice in the same block, this >> will redefine __nr_nodes and fail to compile, no? >> In that case it's better to avoid setting up this trap, IMHO. > >Ah, and it will also generate the following warning, if the use of for_each* >macro is not the first thing after variable declarations, but there's another >statement before: > >warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] > >Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P Hmm... I tried what you suggested, but compiler complains. 'for' loop initial declarations are only allowed in C99 or C11 mode
On 7/14/20 11:57 AM, Wei Yang wrote: > On Tue, Jul 14, 2020 at 11:22:03AM +0200, Vlastimil Babka wrote: >>On 7/14/20 11:13 AM, Vlastimil Babka wrote: >>> On 7/14/20 9:34 AM, Wei Yang wrote: >>>> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >>>> variant, which is not used outside of loop iteration. >>>> >>>> Let's hide this. >>>> >>>> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>>> --- >>>> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >>>> 1 file changed, 20 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>>> index 57ece74e3aae..9c3d15fb317e 100644 >>>> --- a/mm/hugetlb.c >>>> +++ b/mm/hugetlb.c >>>> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >>>> return nid; >>>> } >>>> >>>> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >>>> - for (nr_nodes = nodes_weight(*mask); \ >>>> - nr_nodes > 0 && \ >>>> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >>>> + int __nr_nodes; \ >>>> + for (__nr_nodes = nodes_weight(*mask); \ >>> >>> The problem with this is that if I use the macro twice in the same block, this >>> will redefine __nr_nodes and fail to compile, no? >>> In that case it's better to avoid setting up this trap, IMHO. >> >>Ah, and it will also generate the following warning, if the use of for_each* >>macro is not the first thing after variable declarations, but there's another >>statement before: >> >>warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] >> >>Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P > > Hmm... I tried what you suggested, but compiler complains. > > 'for' loop initial declarations are only allowed in C99 or C11 mode Yes, by "we should switch to C99" I meant that the kernel kbuild system would need to switch. Not a trivial change... Without that, I don't see how your patch is possible to do safely.
On 7/14/20 3:02 AM, Vlastimil Babka wrote: > On 7/14/20 11:57 AM, Wei Yang wrote: >> On Tue, Jul 14, 2020 at 11:22:03AM +0200, Vlastimil Babka wrote: >>> On 7/14/20 11:13 AM, Vlastimil Babka wrote: >>>> On 7/14/20 9:34 AM, Wei Yang wrote: >>>>> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >>>>> variant, which is not used outside of loop iteration. >>>>> >>>>> Let's hide this. >>>>> >>>>> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>>>> --- >>>>> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >>>>> 1 file changed, 20 insertions(+), 18 deletions(-) >>>>> >>>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>>>> index 57ece74e3aae..9c3d15fb317e 100644 >>>>> --- a/mm/hugetlb.c >>>>> +++ b/mm/hugetlb.c >>>>> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >>>>> return nid; >>>>> } >>>>> >>>>> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >>>>> - for (nr_nodes = nodes_weight(*mask); \ >>>>> - nr_nodes > 0 && \ >>>>> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >>>>> + int __nr_nodes; \ >>>>> + for (__nr_nodes = nodes_weight(*mask); \ >>>> >>>> The problem with this is that if I use the macro twice in the same block, this >>>> will redefine __nr_nodes and fail to compile, no? >>>> In that case it's better to avoid setting up this trap, IMHO. >>> >>> Ah, and it will also generate the following warning, if the use of for_each* >>> macro is not the first thing after variable declarations, but there's another >>> statement before: >>> >>> warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] >>> >>> Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P >> >> Hmm... I tried what you suggested, but compiler complains. >> >> 'for' loop initial declarations are only allowed in C99 or C11 mode > > Yes, by "we should switch to C99" I meant that the kernel kbuild system would > need to switch. Not a trivial change... > Without that, I don't see how your patch is possible to do safely. Vlastimil, thanks for pointing out future potential issues with this patch. I likely would have missed that. Wei, thanks for taking the time to put together the patch. However, I tend to agree with Vlastimil's assesment. The cleanup is not worth the risk of running into issues if someone uses multiple instances of the macro.
On Tue, Jul 14, 2020 at 02:12:03PM -0700, Mike Kravetz wrote: >On 7/14/20 3:02 AM, Vlastimil Babka wrote: >> On 7/14/20 11:57 AM, Wei Yang wrote: >>> On Tue, Jul 14, 2020 at 11:22:03AM +0200, Vlastimil Babka wrote: >>>> On 7/14/20 11:13 AM, Vlastimil Babka wrote: >>>>> On 7/14/20 9:34 AM, Wei Yang wrote: >>>>>> The second parameter of for_each_node_mask_to_[alloc|free] is a loop >>>>>> variant, which is not used outside of loop iteration. >>>>>> >>>>>> Let's hide this. >>>>>> >>>>>> Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> >>>>>> --- >>>>>> mm/hugetlb.c | 38 ++++++++++++++++++++------------------ >>>>>> 1 file changed, 20 insertions(+), 18 deletions(-) >>>>>> >>>>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>>>>> index 57ece74e3aae..9c3d15fb317e 100644 >>>>>> --- a/mm/hugetlb.c >>>>>> +++ b/mm/hugetlb.c >>>>>> @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) >>>>>> return nid; >>>>>> } >>>>>> >>>>>> -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ >>>>>> - for (nr_nodes = nodes_weight(*mask); \ >>>>>> - nr_nodes > 0 && \ >>>>>> +#define for_each_node_mask_to_alloc(hs, node, mask) \ >>>>>> + int __nr_nodes; \ >>>>>> + for (__nr_nodes = nodes_weight(*mask); \ >>>>> >>>>> The problem with this is that if I use the macro twice in the same block, this >>>>> will redefine __nr_nodes and fail to compile, no? >>>>> In that case it's better to avoid setting up this trap, IMHO. >>>> >>>> Ah, and it will also generate the following warning, if the use of for_each* >>>> macro is not the first thing after variable declarations, but there's another >>>> statement before: >>>> >>>> warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] >>>> >>>> Instead we should switch to C99 and declare it as "for (int __nr_nodes" :P >>> >>> Hmm... I tried what you suggested, but compiler complains. >>> >>> 'for' loop initial declarations are only allowed in C99 or C11 mode >> >> Yes, by "we should switch to C99" I meant that the kernel kbuild system would >> need to switch. Not a trivial change... >> Without that, I don't see how your patch is possible to do safely. > >Vlastimil, thanks for pointing out future potential issues with this patch. >I likely would have missed that. > >Wei, thanks for taking the time to put together the patch. However, I tend >to agree with Vlastimil's assesment. The cleanup is not worth the risk of >running into issues if someone uses multiple instances of the macro. Yep, thanks all for your feedback. >-- >Mike Kravetz
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 57ece74e3aae..9c3d15fb317e 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1196,17 +1196,19 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed) return nid; } -#define for_each_node_mask_to_alloc(hs, nr_nodes, node, mask) \ - for (nr_nodes = nodes_weight(*mask); \ - nr_nodes > 0 && \ +#define for_each_node_mask_to_alloc(hs, node, mask) \ + int __nr_nodes; \ + for (__nr_nodes = nodes_weight(*mask); \ + __nr_nodes > 0 && \ ((node = hstate_next_node_to_alloc(hs, mask)) || 1); \ - nr_nodes--) + __nr_nodes--) -#define for_each_node_mask_to_free(hs, nr_nodes, node, mask) \ - for (nr_nodes = nodes_weight(*mask); \ - nr_nodes > 0 && \ +#define for_each_node_mask_to_free(hs, node, mask) \ + int __nr_nodes; \ + for (__nr_nodes = nodes_weight(*mask); \ + __nr_nodes > 0 && \ ((node = hstate_next_node_to_free(hs, mask)) || 1); \ - nr_nodes--) + __nr_nodes--) #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE static void destroy_compound_gigantic_page(struct page *page, @@ -1403,7 +1405,7 @@ static void __free_huge_page(struct page *page) * reservation. If the page was associated with a subpool, there * would have been a page reserved in the subpool before allocation * via hugepage_subpool_get_pages(). Since we are 'restoring' the - * reservtion, do not call hugepage_subpool_put_pages() as this will + * reservation, do not call hugepage_subpool_put_pages() as this will * remove the reserved page from the subpool. */ if (!restore_reserve) { @@ -1760,10 +1762,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, nodemask_t *node_alloc_noretry) { struct page *page; - int nr_nodes, node; + int node; gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE; - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { + for_each_node_mask_to_alloc(h, node, nodes_allowed) { page = alloc_fresh_huge_page(h, gfp_mask, node, nodes_allowed, node_alloc_noretry); if (page) @@ -1787,10 +1789,10 @@ static int alloc_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, bool acct_surplus) { - int nr_nodes, node; + int node; int ret = 0; - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { + for_each_node_mask_to_free(h, node, nodes_allowed) { /* * If we're returning unused surplus pages, only examine * nodes with surplus pages. @@ -2481,9 +2483,9 @@ int alloc_bootmem_huge_page(struct hstate *h) int __alloc_bootmem_huge_page(struct hstate *h) { struct huge_bootmem_page *m; - int nr_nodes, node; + int node; - for_each_node_mask_to_alloc(h, nr_nodes, node, &node_states[N_MEMORY]) { + for_each_node_mask_to_alloc(h, node, &node_states[N_MEMORY]) { void *addr; addr = memblock_alloc_try_nid_raw( @@ -2662,17 +2664,17 @@ static inline void try_to_free_low(struct hstate *h, unsigned long count, static int adjust_pool_surplus(struct hstate *h, nodemask_t *nodes_allowed, int delta) { - int nr_nodes, node; + int node; VM_BUG_ON(delta != -1 && delta != 1); if (delta < 0) { - for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { + for_each_node_mask_to_alloc(h, node, nodes_allowed) { if (h->surplus_huge_pages_node[node]) goto found; } } else { - for_each_node_mask_to_free(h, nr_nodes, node, nodes_allowed) { + for_each_node_mask_to_free(h, node, nodes_allowed) { if (h->surplus_huge_pages_node[node] < h->nr_huge_pages_node[node]) goto found;
The second parameter of for_each_node_mask_to_[alloc|free] is a loop variant, which is not used outside of loop iteration. Let's hide this. Signed-off-by: Wei Yang <richard.weiyang@linux.alibaba.com> --- mm/hugetlb.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)