mm: mbind: add hugepage migration code to mbind()

Hello Naoya Horiguchi,

This is a semi-automatic email about new static checker warnings.

The patch 4c5bbbd24ae1: "mm: mbind: add hugepage migration code to mbind()" from Aug 16, 2013, leads to the following Smatch complaint:

mm/mempolicy.c:1199 new_vma_page()
error: we previously assumed 'vma' could be null (see line 1191)

mm/mempolicy.c
1190

  1191		while (vma) {
                       ^^^

Old check.

  1192			address = page_address_in_vma(page, vma);
  1193			if (address != -EFAULT)
  1194				break;
  1195			vma = vma->vm_next;
  1196		}
  1197	
  1198		if (PageHuge(page))
  1199			return alloc_huge_page_noerr(vma, address, 1);
                                                     ^^^

New dereference inside the call to alloc_huge_page_noerr()

1200 /*
1201 * if !vma, alloc_page_vma() will use task or system default policy

regards,
dan carpenter

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi Dan,
(Cc:ed MM maintainers/developers.)

Thanks for reporting.

I think that making alloc_huge_page_noerr() return NULL for !vma is one possible solution. But current code looks strange to me in anther way, and I don't think that considering vma==NULL case is meaningful.

When migrate_pages() is called from do_mbind(), pages in the pagelist are collected via check_range(). And the collected pages certainly belong to some vma. So it seems to me that something wrong should happen in !vma case. So my suggestion is to add BUG_ON(!vma) after the while loop with comments.

Thanks,
Naoya Horiguchi
------------------------------------------------------------------------
From: Naoya Horiguchi <[email protected]>
Date: Mon, 19 Aug 2013 13:23:34 -0400
Subject: [PATCH][mmotm] mbind: add BUG_ON(!vma) in new_vma_page()

new_vma_page() is called only by page migration called from do_mbind(), where pages to be migrated are queued into a pagelist by queue_pages_range(). queue_pages_range() confirms that a queued page belongs to some vma, so !vma case is not supposed to be happen.
This patch adds BUG_ON() to catch this unexpected case.

Dependency:
"mempolicy: rename check_*range to queue_pages_*range" in git//git.cmpxchg.org/linux-mmotm master

---
 mm/mempolicy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index dca5225..43a70dc 100644
@@ -1187,12 +1187,14 @@ static struct page *new_vma_page(struct page *page, unsigned long private, int *
 			break;
 		vma = vma->vm_next;
 	}
+	/*
+	 * queue_pages_range() confirms that @page belongs to some vma,
+	 * so vma shouldn't be NULL.
+	 */
+	BUG_ON(!vma);
 
 	if (PageHuge(page))
 		return alloc_huge_page_noerr(vma, address, 1);
-	/*
-	 * if !vma, alloc_page_vma() will use task or system default policy
-	 */
 	return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
 }
 #else

1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>