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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210323123913.GD1719932@casper.infradead.org>
Date:   Tue, 23 Mar 2021 12:39:13 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Uladzislau Rezki <urezki@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Vlastimil Babka <vbabka@...e.cz>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Nicholas Piggin <npiggin@...il.com>
Subject: Re: [PATCH 2/2] mm/vmalloc: Use kvmalloc to allocate the table of
 pages

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@...il.com>
> 
> Thanks!

Thank you!