Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752451AbbGOAZ4 (ORCPT ); Tue, 14 Jul 2015 20:25:56 -0400 Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:48377 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751813AbbGOAZz (ORCPT ); Tue, 14 Jul 2015 20:25:55 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AHCAAfqKVV/zurLHlbgxOBPYJWpjMBAQEBAQEGmjACAgEBAoFJTQEBAQEBAYELhCMBAQEDAScTHCMFCwgDFAQJJQ8FJQMhE4gmB89BAQEBBwIBHxmGBYUuhQYHhCsFlDKHSYQ8jheKWCZjgSkcgWUsMYJLAQEB Date: Wed, 15 Jul 2015 10:25:40 +1000 From: Dave Chinner To: David Rientjes Cc: Mike Snitzer , Mikulas Patocka , Edward Thornber , linux-kernel@vger.kernel.org, linux-mm@kvack.org, dm-devel@redhat.com, Vivek Goyal , Andrew Morton , Linus Torvalds , "Alasdair G. Kergon" Subject: Re: [PATCH 2/7] mm: introduce kvmalloc and kvmalloc_node Message-ID: <20150715002540.GR3902@dastard> References: <20150707144117.5b38ac38efda238af8a1f536@linux-foundation.org> <20150708161815.bdff609d77868dbdc2e1ce64@linux-foundation.org> <20150714211918.GC7915@redhat.com> <20150714215413.GP3902@dastard> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3375 Lines: 85 On Tue, Jul 14, 2015 at 03:45:40PM -0700, David Rientjes wrote: > On Wed, 15 Jul 2015, Dave Chinner wrote: > > > > Sure, but it's not accomplishing the same thing: things like > > > ext4_kvmalloc() only want to fallback to vmalloc() when high-order > > > allocations fail: the function is used for different sizes. This cannot > > > be converted to kvmalloc_node() since it fallsback immediately when > > > reclaim fails. Same issue with single_file_open() for the seq_file code. > > > We could go through every kmalloc() -> vmalloc() fallback for more > > > examples in the code, but those two instances were the first I looked at > > > and couldn't be converted to kvmalloc_node() without work. > > > > > > > It is always easier to shoehorn utility functions locally within a > > > > subsystem (be it ext4, dm, etc) but once enough do something in a > > > > similar but different way it really should get elevated. > > > > > > > > > > I would argue that > > > > > > void *ext4_kvmalloc(size_t size, gfp_t flags) > > > { > > > void *ret; > > > > > > ret = kmalloc(size, flags | __GFP_NOWARN); > > > if (!ret) > > > ret = __vmalloc(size, flags, PAGE_KERNEL); > > > return ret; > > > } > > > > > > is simple enough that we don't need to convert it to anything. > > > > Except that it will have problems with GFP_NOFS context when the pte > > code inside vmalloc does a GFP_KERNEL allocation. Hence we have > > stuff in other subsystems (such as XFS) where we've noticed lockdep > > whining about this: > > > > Does anyone have an example of ext4_kvmalloc() having a lockdep violation? > Presumably the GFP_NOFS calls to ext4_kvmalloc() will never have > size > (1 << (PAGE_SHIFT + PAGE_ALLOC_COSTLY_ORDER)) so that kmalloc() > above actually never returns NULL and __vmalloc() only gets used for the > ext4_kvmalloc(..., GFP_KERNEL) call. Code inspection is all that is necessary. For example, fs/ext4/resize.c::add_new_gdb() does: 818 n_group_desc = ext4_kvmalloc((gdb_num + 1) * 819 sizeof(struct buffer_head *), 820 GFP_NOFS); I have to assume this was done because resize was failing kmalloc() calls on large filesystems in GFP_NOFS context as the commit message is less than helpful: commit f18a5f21c25707b4fe64b326e2b4d150565e7300 Author: Theodore Ts'o Date: Mon Aug 1 08:45:38 2011 -0400 ext4: use ext4_kvzalloc()/ext4_kvmalloc() for s_group_desc and s_group_info Signed-off-by: "Theodore Ts'o" > It should be fixed, though, probably in the same way as > kmem_zalloc_large() today, but it seems the real fix would be to attack > the whole vmalloc() GFP_KERNEL issue that has been talked about several > times in the past. Then the existing ext4_kvmalloc() implementation > should be fine. Agreed, we really need to ensure that the generic kernel allocation functions follow the context guidelines they are provided by callers. I'm not going to hold my breathe waiting for this to happen, though.... Cheers, Dave. -- Dave Chinner david@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/