weird allocation pattern in alloc_ila_locks

From: Michal Hocko
Date: Fri Jan 06 2017 - 04:52:28 EST


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@xxxxxxxxxx
--
Michal Hocko
SUSE Labs