| Message ID | 20210208083739.33178-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/hugetlb: Remove redundant VM_BUG_ON_PAGE on putback_active_hugepage() | expand |
On 2/8/21 12:37 AM, Miaohe Lin wrote: > PageHead(page) is implicitly checked in set_page_huge_active() via the > PageHeadHuge(page) check. So remove this explicit one. I do not disagree with the code change. However, this commit message is not accurate. set_page_huge_active() no longer exists in the tree you are changing. It was replaced with SetHPageMigratable. Also, the VM_BUG_ON_PAGE(!PageHeadHuge(page), page) was removed in the process. So, there is no redundant check. However, a quick audit of calling code reveals that all callers know they are operating on a hugetlb head page.
Hi: On 2021/2/9 9:26, Mike Kravetz wrote: > On 2/8/21 12:37 AM, Miaohe Lin wrote: >> PageHead(page) is implicitly checked in set_page_huge_active() via the >> PageHeadHuge(page) check. So remove this explicit one. > > I do not disagree with the code change. However, this commit message > is not accurate. set_page_huge_active() no longer exists in the tree > you are changing. It was replaced with SetHPageMigratable. Also, the > VM_BUG_ON_PAGE(!PageHeadHuge(page), page) was removed in the process. > So, there is no redundant check. > > However, a quick audit of calling code reveals that all callers know they > are operating on a hugetlb head page. > So I should change the commit log like: All callers know they are operating on a hugetlb head page. So this VM_BUG_ON_PAGE can't catch anything useful. and send a v2. Right?
On 2/8/21 6:10 PM, Miaohe Lin wrote: > Hi: > On 2021/2/9 9:26, Mike Kravetz wrote: >> On 2/8/21 12:37 AM, Miaohe Lin wrote: >>> PageHead(page) is implicitly checked in set_page_huge_active() via the >>> PageHeadHuge(page) check. So remove this explicit one. >> >> I do not disagree with the code change. However, this commit message >> is not accurate. set_page_huge_active() no longer exists in the tree >> you are changing. It was replaced with SetHPageMigratable. Also, the >> VM_BUG_ON_PAGE(!PageHeadHuge(page), page) was removed in the process. >> So, there is no redundant check. >> >> However, a quick audit of calling code reveals that all callers know they >> are operating on a hugetlb head page. >> > > So I should change the commit log like: > > All callers know they are operating on a hugetlb head page. So this VM_BUG_ON_PAGE > can't catch anything useful. > > and send a v2. Right? Correct,
Hi: On 2021/2/9 11:39, Mike Kravetz wrote: > On 2/8/21 6:10 PM, Miaohe Lin wrote: >> Hi: >> On 2021/2/9 9:26, Mike Kravetz wrote: >>> On 2/8/21 12:37 AM, Miaohe Lin wrote: >>>> PageHead(page) is implicitly checked in set_page_huge_active() via the >>>> PageHeadHuge(page) check. So remove this explicit one. >>> >>> I do not disagree with the code change. However, this commit message >>> is not accurate. set_page_huge_active() no longer exists in the tree >>> you are changing. It was replaced with SetHPageMigratable. Also, the >>> VM_BUG_ON_PAGE(!PageHeadHuge(page), page) was removed in the process. >>> So, there is no redundant check. >>> >>> However, a quick audit of calling code reveals that all callers know they >>> are operating on a hugetlb head page. >>> >> >> So I should change the commit log like: >> >> All callers know they are operating on a hugetlb head page. So this VM_BUG_ON_PAGE >> can't catch anything useful. >> >> and send a v2. Right? > > Correct, > Will do. Thanks a lot.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 6cdb59d8f663..bbbe013a3a2d 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5577,7 +5577,6 @@ bool isolate_huge_page(struct page *page, struct list_head *list) void putback_active_hugepage(struct page *page) { - VM_BUG_ON_PAGE(!PageHead(page), page); spin_lock(&hugetlb_lock); SetHPageMigratable(page); list_move_tail(&page->lru, &(page_hstate(page))->hugepage_activelist);
PageHead(page) is implicitly checked in set_page_huge_active() via the PageHeadHuge(page) check. So remove this explicit one. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/hugetlb.c | 1 - 1 file changed, 1 deletion(-)