Messages in this thread | | | Date | Mon, 9 Nov 2020 17:48:29 +0100 | From | Oscar Salvador <> | Subject | Re: [PATCH v3 04/21] mm/hugetlb: Introduce nr_free_vmemmap_pages in the struct hstate |
| |
On Sun, Nov 08, 2020 at 10:10:56PM +0800, Muchun Song wrote: > +#ifdef CONFIG_HUGETLB_PAGE_FREE_VMEMMAP > +/* > + * There are 512 struct page structs(8 pages) associated with each 2MB > + * hugetlb page. For tail pages, the value of compound_dtor is the same. I gess you meant "For tail pages, the value of compound_head ...", right?
> + * So we can reuse first page of tail page structs. We map the virtual > + * addresses of the remaining 6 pages of tail page structs to the first > + * tail page struct, and then free these 6 pages. Therefore, we need to > + * reserve at least 2 pages as vmemmap areas. > + */ > +#define RESERVE_VMEMMAP_NR 2U > + > +static void __init hugetlb_vmemmap_init(struct hstate *h) > +{ > + unsigned int order = huge_page_order(h); > + unsigned int vmemmap_pages; > + > + vmemmap_pages = ((1 << order) * sizeof(struct page)) >> PAGE_SHIFT; > + /* > + * The head page and the first tail page not free to buddy system,
"The head page and the first tail page are not to be freed to..." better?
> + * the others page will map to the first tail page. So there are > + * (@vmemmap_pages - RESERVE_VMEMMAP_NR) pages can be freed. ^^^ that
> + else > + h->nr_free_vmemmap_pages = 0;
I would specify that this is not expected to happen. (At least I could not come up with a real scenario unless the system is corrupted) So, I would drop a brief comment pointing out that it is only a safety net.
Unrelated to this patch but related in general, I am not sure about Mike but would it be cleaner to move all the vmemmap functions to hugetlb_vmemmap.c? hugetlb code is quite tricky, so I am not sure about stuffing more code in there.
-- Oscar Salvador SUSE L3
| |