Messages in this thread | | | Subject | Re: [PATCH v2 1/1] mm/hugetlb: fix memory offline with hugepage size > memory block size | From | Rui Teng <> | Date | Wed, 21 Sep 2016 21:17:29 +0800 |
| |
On 9/21/16 8:35 PM, Gerald Schaefer wrote: > dissolve_free_huge_pages() will either run into the VM_BUG_ON() or a > list corruption and addressing exception when trying to set a memory > block offline that is part (but not the first part) of a hugetlb page > with a size > memory block size. > > When no other smaller hugetlb page sizes are present, the VM_BUG_ON() > will trigger directly. In the other case we will run into an addressing > exception later, because dissolve_free_huge_page() will not work on the > head page of the compound hugetlb page which will result in a NULL > hstate from page_hstate(). > > To fix this, first remove the VM_BUG_ON() because it is wrong, and then > use the compound head page in dissolve_free_huge_page(). > > Also change locking in dissolve_free_huge_page(), so that it only takes > the lock when actually removing a hugepage. > > Signed-off-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> > --- > Changes in v2: > - Update comment in dissolve_free_huge_pages() > - Change locking in dissolve_free_huge_page() > > mm/hugetlb.c | 31 +++++++++++++++++++------------ > 1 file changed, 19 insertions(+), 12 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 87e11d8..1522af8 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1441,23 +1441,30 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, > */ > static void dissolve_free_huge_page(struct page *page) > { > + struct page *head = compound_head(page); > + struct hstate *h; > + int nid; > + > + if (page_count(head)) > + return; > + > + h = page_hstate(head); > + nid = page_to_nid(head); > + > spin_lock(&hugetlb_lock); > - if (PageHuge(page) && !page_count(page)) { > - struct hstate *h = page_hstate(page); > - int nid = page_to_nid(page); > - list_del(&page->lru); > - h->free_huge_pages--; > - h->free_huge_pages_node[nid]--; > - h->max_huge_pages--; > - update_and_free_page(h, page); > - } > + list_del(&head->lru); > + h->free_huge_pages--; > + h->free_huge_pages_node[nid]--; > + h->max_huge_pages--; > + update_and_free_page(h, head); > spin_unlock(&hugetlb_lock); > } > > /* > * Dissolve free hugepages in a given pfn range. Used by memory hotplug to > * make specified memory blocks removable from the system. > - * Note that start_pfn should aligned with (minimum) hugepage size. > + * Note that this will dissolve a free gigantic hugepage completely, if any > + * part of it lies within the given range. > */ > void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn) > { > @@ -1466,9 +1473,9 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn) > if (!hugepages_supported()) > return; > > - VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << minimum_order)); > for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << minimum_order) > - dissolve_free_huge_page(pfn_to_page(pfn)); > + if (PageHuge(pfn_to_page(pfn))) > + dissolve_free_huge_page(pfn_to_page(pfn)); How many times will dissolve_free_huge_page() be invoked in this loop? For each pfn, it will be converted to the head page, and then the list will be deleted repeatedly. > } > > /* >
| |