From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034899AbdAFJwY (ORCPT ); Fri, 6 Jan 2017 04:52:24 -0500 Received: from mx2.suse.de ([195.135.220.15]:36203 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936568AbdAFJv1 (ORCPT ); Fri, 6 Jan 2017 04:51:27 -0500 Date: Fri, 6 Jan 2017 10:51:15 +0100 From: Michal Hocko To: Tom Herbert Cc: linux-mm@kvack.org, LKML Subject: weird allocation pattern in alloc_ila_locks Message-ID: <20170106095115.GG5556@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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