I know almost nothing about thp, unlikely I understand this patch
correctly...
But, afaics, the main idea is that until we have mm->thp_threshold
faults we install the tail pages of temp_hugepage->page as a normal
anonymous page, then we actually add the Head/Tail metadata and add
the necessary huge_pmd/etc.
I simply can't understand how this can work until make_compound_page()
is called. Just for example, what happens after sys_munmap() ? If
nothing else, who will find and free temp_hugepage connected to this
area? Or, what if sys_mremap() moves this vma? find_pmd_mm_freelist()
won't find the right temp_thp after that? Or split_vma, etc.
And make_compound_page() itself looks suspicious,
On 12/12, Alex Thorlton wrote:
>
> +void make_compound_page(struct page *page, unsigned long order)
> +{
> + int i, max_count = 0, max_mapcount = 0;
> + int nr_pages = 1 << order;
> +
> + set_compound_page_dtor(page, free_compound_page);
> + set_compound_order(page, order);
> +
> + __SetPageHead(page);
> +
> + /*
> + * we clear all the mappings here, so we have to remember to set
> + * them back up!
> + */
> + page->mapping = NULL;
> +
> + max_count = (int) atomic_read(&page->_count);
> + max_mapcount = (int) atomic_read(&page->_mapcount);
> +
> + for (i = 1; i < nr_pages; i++) {
> + int cur_count, cur_mapcount;
> + struct page *p = page + i;
> + p->flags = 0; /* this seems dumb */
> + __SetPageTail(p);
Just for example, what if put_page(p) or get_page(p) is called after
__SetPageTail() ?
Afaics, this page was already visible to, say, get_user_pages() and
it can have external references.
In fact everything else looks suspicious too but let me repeat that
I do not really understand this code. So please do not count this as
review, but perhaps the changelog should tell more to explain what
this patch actually does and how this all should work?
Oleg.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>