Messages in this thread |  | | Subject | Re: [PATCH v11 6/9] hugetlb_cgroup: support noreserve mappings | From | Mike Kravetz <> | Date | Tue, 11 Feb 2020 13:51:33 -0800 |
| |
On 2/11/20 1:35 PM, Mina Almasry wrote: > On Thu, Feb 6, 2020 at 2:31 PM Mike Kravetz <mike.kravetz@oracle.com> wrote: >> On 2/3/20 3:22 PM, Mina Almasry wrote: >>> +++ b/mm/hugetlb.c >>> @@ -1339,6 +1339,9 @@ static void __free_huge_page(struct page *page) >>> clear_page_huge_active(page); >>> hugetlb_cgroup_uncharge_page(hstate_index(h), pages_per_huge_page(h), >>> page, false); >>> + hugetlb_cgroup_uncharge_page(hstate_index(h), pages_per_huge_page(h), >>> + page, true); >>> + >> >> When looking at the code without change markings, the two above lines >> look so similar my first thought is there must be a mistake. >> >> A suggestion for better code readability: >> - hugetlb_cgroup_uncharge_page could just take "struct hstate *h" and >> get both hstate_index(h) and pages_per_huge_page(h). >> - Perhaps make hugetlb_cgroup_uncharge_page and >> hugetlb_cgroup_uncharge_page_rsvd be wrappers around a common routine. >> Then the above would look like: >> >> hugetlb_cgroup_uncharge_page(h, page); >> hugetlb_cgroup_uncharge_page_rsvd(h, page); >> > > I did modify the interfaces to this, as it's much better for > readability indeed. Unfortunately the patch the adds interfaces > probably needs a re-review now as it's changed quite a bit, I did not > carry your or David's Reviewed-by.
No worries. Happy to review again.
-- Mike Kravetz
|  |