[bug report] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()

[bug report] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()

[Dan Carpenter]

Hello Naoya Horiguchi,

The patch 3b325af8e5fe: "mm/hwpoison: fix race between hugetlb
free/demotion and memory_failure_hugetlb()" from Apr 5, 2022, leads
to the following Smatch static checker warning:

	mm/memory-failure.c:1537 __get_huge_page_for_hwpoison()
	warn: sleeping in atomic context

mm/memory-failure.c
    1515 int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
    1516 {
    1517         struct page *page = pfn_to_page(pfn);
    1518         struct page *head = compound_head(page);
    1519         int ret = 2;        /* fallback to normal page handling */
    1520         bool count_increased = false;
    1521 
    1522         if (!PageHeadHuge(head))
    1523                 goto out;
    1524 
    1525         if (flags & MF_COUNT_INCREASED) {
    1526                 ret = 1;
    1527                 count_increased = true;
    1528         } else if (HPageFreed(head) || HPageMigratable(head)) {
    1529                 ret = get_page_unless_zero(head);
    1530                 if (ret)
    1531                         count_increased = true;
    1532         } else {
    1533                 ret = -EBUSY;
    1534                 goto out;
    1535         }
    1536 
--> 1537         lock_page(head);
                 ^^^^^^^^^^^^^^^
Sleeps.  But this is called from get_huge_page_for_hwpoison() with
a spinlock held.

    1538 
    1539         if (hwpoison_filter(page)) {
    1540                 ret = -EOPNOTSUPP;
    1541                 goto unlock;
    1542         }
    1543 
    1544         if (TestSetPageHWPoison(head)) {
    1545                 ret = -EHWPOISON;
    1546                 goto unlock;
    1547         }
    1548 
    1549         /* keep locking page. */
    1550         return ret;
    1551 unlock:
    1552         unlock_page(head);
    1553         if (count_increased)
    1554                 put_page(head);
    1555 out:
    1556         return ret;
    1557 }

regards,
dan carpenter

Re: [bug report] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()

[Naoya Horiguchi]

On Wed, Apr 06, 2022 at 09:38:09AM +0300, Dan Carpenter wrote:
> Hello Naoya Horiguchi,
> 
> The patch 3b325af8e5fe: "mm/hwpoison: fix race between hugetlb
> free/demotion and memory_failure_hugetlb()" from Apr 5, 2022, leads
> to the following Smatch static checker warning:
> 
> 	mm/memory-failure.c:1537 __get_huge_page_for_hwpoison()
> 	warn: sleeping in atomic context
> 
> mm/memory-failure.c
>     1515 int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
>     1516 {
>     1517         struct page *page = pfn_to_page(pfn);
>     1518         struct page *head = compound_head(page);
>     1519         int ret = 2;        /* fallback to normal page handling */
>     1520         bool count_increased = false;
>     1521 
>     1522         if (!PageHeadHuge(head))
>     1523                 goto out;
>     1524 
>     1525         if (flags & MF_COUNT_INCREASED) {
>     1526                 ret = 1;
>     1527                 count_increased = true;
>     1528         } else if (HPageFreed(head) || HPageMigratable(head)) {
>     1529                 ret = get_page_unless_zero(head);
>     1530                 if (ret)
>     1531                         count_increased = true;
>     1532         } else {
>     1533                 ret = -EBUSY;
>     1534                 goto out;
>     1535         }
>     1536 
> --> 1537         lock_page(head);
>                  ^^^^^^^^^^^^^^^
> Sleeps.  But this is called from get_huge_page_for_hwpoison() with
> a spinlock held.

Thank you for checking/reporting, I'll move this lock_page() outside hugetlb_lock.

- Naoya Horiguchi

> 
>     1538 
>     1539         if (hwpoison_filter(page)) {
>     1540                 ret = -EOPNOTSUPP;
>     1541                 goto unlock;
>     1542         }
>     1543 
>     1544         if (TestSetPageHWPoison(head)) {
>     1545                 ret = -EHWPOISON;
>     1546                 goto unlock;
>     1547         }
>     1548 
>     1549         /* keep locking page. */
>     1550         return ret;
>     1551 unlock:
>     1552         unlock_page(head);
>     1553         if (count_increased)
>     1554                 put_page(head);
>     1555 out:
>     1556         return ret;
>     1557 }
> 
> regards,
> dan carpenter
>