Re: [PATCH 2/2] mm/vmalloc: Use kvmalloc to allocate the table of pages

From: Matthew Wilcox
Date: Tue Mar 23 2021 - 08:41:08 EST


On Tue, Mar 23, 2021 at 01:04:36PM +0100, Uladzislau Rezki wrote:
> On Mon, Mar 22, 2021 at 11:03:11PM +0000, Matthew Wilcox wrote:
> > I suspect the vast majority of the time is spent calling alloc_pages_node()
> > 1024 times. Have you looked at Mel's patch to do ... well, exactly what
> > vmalloc() wants?
> >
> <snip>
> - __vmalloc_node_range
> - 45.25% __alloc_pages_nodemask
> - 37.59% get_page_from_freelist
[...]
> - 44.61% 0xffffffffc047348d
> - __vunmap
> - 35.56% free_unref_page

Hmm! I hadn't been thinking about the free side of things.
Does this make a difference?

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 4f5f8c907897..61d5b769fea0 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2277,16 +2277,8 @@ static void __vunmap(const void *addr, int deallocate_pages)
vm_remove_mappings(area, deallocate_pages);

if (deallocate_pages) {
- int i;
-
- for (i = 0; i < area->nr_pages; i++) {
- struct page *page = area->pages[i];
-
- BUG_ON(!page);
- __free_pages(page, 0);
- }
+ release_pages(area->pages, area->nr_pages);
atomic_long_sub(area->nr_pages, &nr_vmalloc_pages);
-
kvfree(area->pages);
}

release_pages does a bunch of checks that are unnecessary ... we could
probably just do:

LIST_HEAD(pages_to_free);

for (i = 0; i < area->nr_pages; i++) {
struct page *page = area->pages[i];
if (put_page_testzero(page))
list_add(&page->lru, &pages_to_free);
}
free_unref_page_list(&pages_to_free);

but let's see if the provided interface gets us the performance we want.

> Reviewed-by: Uladzislau Rezki (Sony) <urezki@xxxxxxxxx>
>
> Thanks!

Thank you!