| Message ID | 20210111210152.118394-4-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | create hugetlb flags to consolidate state | expand |
On Mon, Jan 11, 2021 at 01:01:52PM -0800, Mike Kravetz wrote: > Use new hugetlb specific flag HPageTempSurplus to replace the > PageHugeTemporary() interfaces. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- > mm/hugetlb.c | 38 +++++++++----------------------------- > 1 file changed, 9 insertions(+), 29 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 34ce82f4823c..949e1f987319 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -58,6 +58,7 @@ static unsigned long hugetlb_cma_size __initdata; > enum htlb_page_flags { > HPAGE_RestoreReserve = 0, > HPAGE_Migratable, > + HPAGE_TempSurplus, > }; > > /* > @@ -99,6 +100,7 @@ void ClearHPage##flname(struct page *page) \ > > HPAGEFLAG(RestoreReserve) > EXT_HPAGEFLAG(Migratable) > +HPAGEFLAG(TempSurplus) Would HPAGE_Temporary/Temporary not be a better fit? The point about current PageHugeTemporary is that these pages are temporary as they do not belong to the pool and will vanish once the last reference gets drop. We do not really care that much about the surplus part? Besides, alloc_migrate_huge_page seems to not want to thread these pages as surplus. Also, I would add a comment either next to each flag or above the enum htlb_page_flags (probably the latter) with a brief explanation of each flag. Besides that, it looks fine to me. Here I do not see the same problem in stripping the PageHuge check in PageHugeTemporary, as I did in previous patch, because all callers of it make sure they operate on a hugetlb page.
On 1/15/21 2:16 AM, Oscar Salvador wrote: > On Mon, Jan 11, 2021 at 01:01:52PM -0800, Mike Kravetz wrote: >> Use new hugetlb specific flag HPageTempSurplus to replace the >> PageHugeTemporary() interfaces. >> >> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> >> --- >> mm/hugetlb.c | 38 +++++++++----------------------------- >> 1 file changed, 9 insertions(+), 29 deletions(-) >> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 34ce82f4823c..949e1f987319 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -58,6 +58,7 @@ static unsigned long hugetlb_cma_size __initdata; >> enum htlb_page_flags { >> HPAGE_RestoreReserve = 0, >> HPAGE_Migratable, >> + HPAGE_TempSurplus, >> }; >> >> /* >> @@ -99,6 +100,7 @@ void ClearHPage##flname(struct page *page) \ >> >> HPAGEFLAG(RestoreReserve) >> EXT_HPAGEFLAG(Migratable) >> +HPAGEFLAG(TempSurplus) > > Would HPAGE_Temporary/Temporary not be a better fit? Yes it would. > The point about current PageHugeTemporary is that these pages are temporary as > they do not belong to the pool and will vanish once the last reference gets drop. > We do not really care that much about the surplus part? > > Besides, alloc_migrate_huge_page seems to not want to thread these pages as surplus. > All correct, not sure why I was thinking 'surplus' when naming the flag. > Also, I would add a comment either next to each flag or above > the enum htlb_page_flags (probably the latter) with a brief explanation > of each flag. Will do.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 34ce82f4823c..949e1f987319 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -58,6 +58,7 @@ static unsigned long hugetlb_cma_size __initdata; enum htlb_page_flags { HPAGE_RestoreReserve = 0, HPAGE_Migratable, + HPAGE_TempSurplus, }; /* @@ -99,6 +100,7 @@ void ClearHPage##flname(struct page *page) \ HPAGEFLAG(RestoreReserve) EXT_HPAGEFLAG(Migratable) +HPAGEFLAG(TempSurplus) /* * hugetlb page subpool pointer located in hpage[1].private @@ -1398,28 +1400,6 @@ struct hstate *size_to_hstate(unsigned long size) return NULL; } -/* - * Internal hugetlb specific page flag. Do not use outside of the hugetlb - * code - */ -static inline bool PageHugeTemporary(struct page *page) -{ - if (!PageHuge(page)) - return false; - - return (unsigned long)page[2].mapping == -1U; -} - -static inline void SetPageHugeTemporary(struct page *page) -{ - page[2].mapping = (void *)-1U; -} - -static inline void ClearPageHugeTemporary(struct page *page) -{ - page[2].mapping = NULL; -} - static void __free_huge_page(struct page *page) { /* @@ -1467,9 +1447,9 @@ static void __free_huge_page(struct page *page) if (restore_reserve) h->resv_huge_pages++; - if (PageHugeTemporary(page)) { + if (HPageTempSurplus(page)) { list_del(&page->lru); - ClearPageHugeTemporary(page); + ClearHPageTempSurplus(page); update_and_free_page(h, page); } else if (h->surplus_huge_pages_node[nid]) { /* remove the page from active list */ @@ -1883,7 +1863,7 @@ static struct page *alloc_surplus_huge_page(struct hstate *h, gfp_t gfp_mask, * codeflow */ if (h->surplus_huge_pages >= h->nr_overcommit_huge_pages) { - SetPageHugeTemporary(page); + SetHPageTempSurplus(page); spin_unlock(&hugetlb_lock); put_page(page); return NULL; @@ -1914,7 +1894,7 @@ static struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask, * We do not account these pages as surplus because they are only * temporary and will be released properly on the last reference */ - SetPageHugeTemporary(page); + SetHPageTempSurplus(page); return page; } @@ -5612,12 +5592,12 @@ void move_hugetlb_state(struct page *oldpage, struct page *newpage, int reason) * here as well otherwise the global surplus count will not match * the per-node's. */ - if (PageHugeTemporary(newpage)) { + if (HPageTempSurplus(newpage)) { int old_nid = page_to_nid(oldpage); int new_nid = page_to_nid(newpage); - SetPageHugeTemporary(oldpage); - ClearPageHugeTemporary(newpage); + SetHPageTempSurplus(oldpage); + ClearHPageTempSurplus(newpage); spin_lock(&hugetlb_lock); if (h->surplus_huge_pages_node[old_nid]) {
Use new hugetlb specific flag HPageTempSurplus to replace the PageHugeTemporary() interfaces. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> --- mm/hugetlb.c | 38 +++++++++----------------------------- 1 file changed, 9 insertions(+), 29 deletions(-)