Re: [bug report] lib/test_hmm: add zone device private THP test infrastructure

[Balbir Singh <balbirs@nvidia.com>]

On 9/15/25 15:50, Dan Carpenter wrote:
> Hello Balbir Singh,
> 
> Commit 6098c878b175 ("lib/test_hmm: add zone device private THP test
> infrastructure") from Sep 8, 2025 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	lib/test_hmm.c:721 dmirror_migrate_alloc_and_copy()
> 	error: we previously assumed 'spage' could be null (see line 718)
> 
> lib/test_hmm.c
>     675 static void dmirror_migrate_alloc_and_copy(struct migrate_vma *args,
>     676                                            struct dmirror *dmirror)
>     677 {
>     678         const unsigned long *src = args->src;
>     679         unsigned long *dst = args->dst;
>     680         unsigned long addr;
>     681 
>     682         for (addr = args->start; addr < args->end; ) {
>     683                 struct page *spage;
>     684                 struct page *dpage;
>     685                 struct page *rpage;
>     686                 bool is_large = *src & MIGRATE_PFN_COMPOUND;
>     687                 int write = (*src & MIGRATE_PFN_WRITE) ? MIGRATE_PFN_WRITE : 0;
>     688                 unsigned long nr = 1;
>     689 
>     690                 if (!(*src & MIGRATE_PFN_MIGRATE))
>     691                         goto next;
>     692 
>     693                 /*
>     694                  * Note that spage might be NULL which is OK since it is an
>     695                  * unallocated pte_none() or read-only zero page.
>     696                  */
>     697                 spage = migrate_pfn_to_page(*src);
>     698                 if (WARN(spage && is_zone_device_page(spage),
>     699                      "page already in device spage pfn: 0x%lx\n",
>     700                      page_to_pfn(spage)))
>     701                         goto next;
>     702 
>     703                 if (dmirror->flags & HMM_DMIRROR_FLAG_FAIL_ALLOC) {
>     704                         dmirror->flags &= ~HMM_DMIRROR_FLAG_FAIL_ALLOC;
>     705                         dpage = NULL;
>     706                 } else
>     707                         dpage = dmirror_devmem_alloc_page(dmirror, is_large);
>     708 
>     709                 if (!dpage) {
>     710                         struct folio *folio;
>     711                         unsigned long i;
>     712                         unsigned long spfn = *src >> MIGRATE_PFN_SHIFT;
>     713                         struct page *src_page;
>     714 
>     715                         if (!is_large)
>     716                                 goto next;
>     717 
>     718                         if (!spage && is_large) {
>                                     ^^^^^^    ^^^^^^^^
> If spage is NULL but is_large is false

Isn't is_large being false ruled out two lines above?

> 
>     719                                 nr = HPAGE_PMD_NR;
>     720                         } else {
> --> 721                                 folio = page_folio(spage);
>                                                            ^^^^^
> the this will crash.
> 
>     722                                 nr = folio_nr_pages(folio);
>     723                         }
>     724 
>     725                         for (i = 0; i < nr && addr < args->end; i++) {
> 
> regards,
> dan carpenter

Thanks,
Balbir