On 11/28/22 11:40 PM, Rasmus Villemoes wrote:
On 29/11/2022 05.57, Dan Carpenter wrote:
On Mon, Nov 28, 2022 at 02:34:54PM -0800, Sidhartha Kumar wrote:
Hello,
One of my patches in linux-next was flagged as a Unverified Error/Warning
with the following warning[1]:
mm/hugetlb.c:2073 alloc_pool_huge_page() error: uninitialized symbol
'folio'.
The relevant change is:
- struct page *page;
+ struct folio *folio;
int nr_nodes, node;
gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) {
- page = alloc_fresh_huge_page(h, gfp_mask, node, nodes_allowed,
- node_alloc_noretry);
- if (page)
+ folio = alloc_fresh_hugetlb_folio(h, gfp_mask, node,
+ nodes_allowed, node_alloc_noretry);
+ if (folio)
break;
}
- if (!page)
+ if (!folio)
It looks like I can initialize folio to NULL to avoid this error but I'm not
sure how this would cause a regression as previously the page variable was
unitialized as well. Please let me know if I am missing something in my
patch or if this should be ignored.
Both the original and the new code trigger a Smatch warning. I don't
know why the kbuild-bot marks this as a new warning. Possibly that's
because the variable name has changed? The kbuild-bot is run by Intel.
The problem is obviously that Smatch doesn't know that we always enter
the loop. There are hack arounds that I could do for this, but
sometimes we dont' actually enter the loop so changing this will hide
bugs... I am conflicted on this.
It seems that a reasonably clean way to deal with this, since there's
not a lot of code following the loop, is to simply move that code into
the "if (page/folio)" branch. I.e. change the tail to
if (page) {
free_huge_page(page); /* free it into the hugepage allocator */
return 1;
}
} // this is the end of loop
return 0;
or however it looks after the above change.
Thanks for the suggestion, this looks like a good way to avoid the
smatch warning and I'll add it to the next version of my patch.
Thanks.
Sidhartha Kumar
Rasmus
