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
