Re: [linux-next:master 7944/8232] mm/hugetlb.c:2677 gather_surplus_pages() error: uninitialized symbol 'folio'.

[Vishal Moola <vishal.moola@gmail.com>]

On Mon, Jul 01, 2024 at 05:49:34PM +0200, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   df9574a57d02b265322e77fb8628d4d33641dda9
> commit: 1cb6271d927cdb448a6a2794291c5405f1effa76 [7944/8232] hugetlb: force allocating surplus hugepages on mempolicy allowed nodes
> config: i386-randconfig-141-20240627 (https://download.01.org/0day-ci/archive/20240627/202406270727.F4yNrBsh-lkp@intel.com/config)
> compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202406270727.F4yNrBsh-lkp@intel.com/
> 
> smatch warnings:
> mm/hugetlb.c:2677 gather_surplus_pages() error: uninitialized symbol 'folio'.
> 
> vim +/folio +2677 mm/hugetlb.c
> 
> 0a4f3d1bb91cac Liu Xiang               2020-12-14  2644  static int gather_surplus_pages(struct hstate *h, long delta)
> 1b2a1e7bb9ce99 Jules Irenge            2020-04-06  2645  	__must_hold(&hugetlb_lock)
> e4e574b767ba63 Adam Litke              2007-10-16  2646  {
> 3466534131b28e Miaohe Lin              2022-09-01  2647  	LIST_HEAD(surplus_list);
> 454a00c40a21c5 Matthew Wilcox (Oracle  2023-08-16  2648) 	struct folio *folio, *tmp;
> 0a4f3d1bb91cac Liu Xiang               2020-12-14  2649  	int ret;
> 0a4f3d1bb91cac Liu Xiang               2020-12-14  2650  	long i;
> 0a4f3d1bb91cac Liu Xiang               2020-12-14  2651  	long needed, allocated;
> 28073b02bfaaed Hillf Danton            2012-03-21  2652  	bool alloc_ok = true;
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2653  	int node;
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2654  	nodemask_t *mbind_nodemask = policy_mbind_nodemask(htlb_alloc_mask(h));
> e4e574b767ba63 Adam Litke              2007-10-16  2655  
> 9487ca60fd7fa2 Mike Kravetz            2021-05-04  2656  	lockdep_assert_held(&hugetlb_lock);
> a5516438959d90 Andi Kleen              2008-07-23  2657  	needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
> ac09b3a15154af Adam Litke              2008-03-04  2658  	if (needed <= 0) {
> a5516438959d90 Andi Kleen              2008-07-23  2659  		h->resv_huge_pages += delta;
> e4e574b767ba63 Adam Litke              2007-10-16  2660  		return 0;
> ac09b3a15154af Adam Litke              2008-03-04  2661  	}
> e4e574b767ba63 Adam Litke              2007-10-16  2662  
> e4e574b767ba63 Adam Litke              2007-10-16  2663  	allocated = 0;
> e4e574b767ba63 Adam Litke              2007-10-16  2664  
> e4e574b767ba63 Adam Litke              2007-10-16  2665  	ret = -ENOMEM;
> e4e574b767ba63 Adam Litke              2007-10-16  2666  retry:
> db71ef79b59bb2 Mike Kravetz            2021-05-04  2667  	spin_unlock_irq(&hugetlb_lock);
> e4e574b767ba63 Adam Litke              2007-10-16  2668  	for (i = 0; i < needed; i++) {
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2669  		for_each_node_mask(node, cpuset_current_mems_allowed) {

Smatch might be concerned about us skipping over this
for_each_node_mask()? It appears to be possible if we have 1 non-empty
Numa node. 

> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2670  			if (!mbind_nodemask || node_isset(node, *mbind_nodemask)) {

Alternatively it might be worried about us skipping the folio assignment
during each iteration due to the if statement here.

> 3a740e8bb56ef7 Sidhartha Kumar         2023-01-13  2671  				folio = alloc_surplus_hugetlb_folio(h, htlb_alloc_mask(h),
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2672  						node, NULL);
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2673  				if (folio)
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2674  					break;
> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2675  			}
> 
> folio is uninitialized if everything is set, I guess.  Not sure if that
> is possible or not.

I'm not familiar enough with NUMA to know whether its possible or not,
but it shouldn't hurt much to initialize the folio.

> 1cb6271d927cdb Aristeu Rozanski        2024-06-21  2676  		}
> 3a740e8bb56ef7 Sidhartha Kumar         2023-01-13 @2677  		if (!folio) {
> 28073b02bfaaed Hillf Danton            2012-03-21  2678  			alloc_ok = false;
> 28073b02bfaaed Hillf Danton            2012-03-21  2679  			break;
> 28073b02bfaaed Hillf Danton            2012-03-21  2680  		}
> 3a740e8bb56ef7 Sidhartha Kumar         2023-01-13  2681  		list_add(&folio->lru, &surplus_list);

If getting here with an uninitialized folio is possible, this could be
bad.

> 69ed779a1454d9 David Rientjes          2017-07-10  2682  		cond_resched();
> e4e574b767ba63 Adam Litke              2007-10-16  2683  	}
> 28073b02bfaaed Hillf Danton            2012-03-21  2684  	allocated += i;
> e4e574b767ba63 Adam Litke              2007-10-16  2685  
> e4e574b767ba63 Adam Litke              2007-10-16  2686  	/*
> e4e574b767ba63 Adam Litke              2007-10-16  2687  	 * After retaking hugetlb_lock, we need to recalculate 'needed'
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>