From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935519AbeE2OnZ (ORCPT ); Tue, 29 May 2018 10:43:25 -0400 Received: from mx2.suse.de ([195.135.220.15]:54156 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935077AbeE2OnU (ORCPT ); Tue, 29 May 2018 10:43:20 -0400 Date: Tue, 29 May 2018 16:43:17 +0200 From: Michal Hocko To: Linus Torvalds Cc: Davidlohr Bueso , Andrew Morton , Thomas Graf , Herbert Xu , Manfred Spraul , guillaume.knispel@supersonicimagine.com, Linux API , Linux Kernel Mailing List , Davidlohr Bueso Subject: Re: [PATCH 3/6] lib/bucket_locks: use kvmalloc_array() Message-ID: <20180529144317.GA20910@dhcp22.suse.cz> References: <20180524211135.27760-1-dave@stgolabs.net> <20180524211135.27760-4-dave@stgolabs.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.5 (2018-04-13) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 24-05-18 14:37:36, Linus Torvalds wrote: > On Thu, May 24, 2018 at 2:28 PM Davidlohr Bueso wrote: > > > if (gfpflags_allow_blocking(gfp)) > > - tlocks = kvmalloc(size * sizeof(spinlock_t), gfp); > > + tlocks = kvmalloc_array(size, sizeof(spinlock_t), > gfp); > > else > > tlocks = kmalloc_array(size, sizeof(spinlock_t), > gfp); > > Side note: how about we just move that "gfpflags_allow_blocking()" into > kvmalloc() instead, and make kvmalloc() generally usable? > > Now we have that really odd situation where kvmalloc() takes gfp flags, but > to quote the comment: > > * Any use of gfp flags outside of GFP_KERNEL should be consulted with mm > people. > > and the code: > > /* > * vmalloc uses GFP_KERNEL for some internal allocations (e.g page > tables) > * so the given set of flags has to be compatible. > */ > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > which isn't really all that helpful. Do mm people really want to be > consulted about random uses? The purpose was to have a clean usage base after the conversion. If we are growing a non-trivial use base which wants to use GFP_NOWAIT semantic then sure we can make kvmalloc never fallback to vmallock. But see below... > Maybe we could just make the rule for kvmalloc() be to only fall back on > vmalloc for allocations that are > > - larger than page size > > - blocking and allow GFP_KERNEL (so basically that WARN_ON_ONCE() logic in > kvmalloc_node). > > Hmm? Isn't that what everybody really *wants* kvmalloc() and friends to do? ... Well, there are users who would like to use kvmalloc for GFP_NOFS/GFP_NOIO context. Do we want them to fail more likely for larger order rather than have them fixed (to either drop the NOFS because it just has been blindly copied from a different code without too much thinking or use the scope NOFS/NOIO API)? A warn_on tends to be rather harsh but effective way to push maintainers fix their broken code... -- Michal Hocko SUSE Labs