linux-kernel - weird allocation pattern in alloc_ila

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20170106095115.GG5556@dhcp22.suse.cz>
Date:   Fri, 6 Jan 2017 10:51:15 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Tom Herbert <tom@...bertland.com>
Cc:     linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>
Subject: weird allocation pattern in alloc_ila_locks

Hi Tom,
I am currently looking at kmalloc with vmalloc fallback users [1]
and came across alloc_ila_locks which is using a pretty unusual
allocation pattern - it seems to be a c&p alloc_bucket_locks which
is doing a similar thing - except it has to support GFP_ATOMIC.

I am really wondering what is the point of 
#ifdef CONFIG_NUMA
		if (size * sizeof(spinlock_t) > PAGE_SIZE)
			ilan->locks = vmalloc(size * sizeof(spinlock_t));
		else
#endif

there doesn't seem to be any NUMA awareness in the ifdef code so I can
only assume that the intention is to reflect that NUMA machines tend to
have more CPUs. On the other hand nr_pcpus is limited to 32 so this
doesn't seem to be the case here...
Can we just get rid of this ugly and confusing code and do something as
simple as
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index af8f52ee7180..1d86ceae61b3 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -41,13 +41,11 @@ static int alloc_ila_locks(struct ila_net *ilan)
 	size = roundup_pow_of_two(nr_pcpus * LOCKS_PER_CPU);
 
 	if (sizeof(spinlock_t) != 0) {
-#ifdef CONFIG_NUMA
-		if (size * sizeof(spinlock_t) > PAGE_SIZE)
-			ilan->locks = vmalloc(size * sizeof(spinlock_t));
-		else
-#endif
 		ilan->locks = kmalloc_array(size, sizeof(spinlock_t),
-					    GFP_KERNEL);
+					    GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
+		if (!ilan->locks)
+			ilan->locks = vmalloc(size * sizeof(spinlock_t));
+
 		if (!ilan->locks)
 			return -ENOMEM;
 		for (i = 0; i < size; i++)

which I would then simply turn into kvmalloc()?


[1] http://lkml.kernel.org/r/20170102133700.1734-1-mhocko@kernel.org
-- 
Michal Hocko
SUSE Labs