Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754041AbbGNVTU (ORCPT ); Tue, 14 Jul 2015 17:19:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39435 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753620AbbGNVTT (ORCPT ); Tue, 14 Jul 2015 17:19:19 -0400 Date: Tue, 14 Jul 2015 17:19:18 -0400 From: Mike Snitzer To: David Rientjes Cc: 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: <20150714211918.GC7915@redhat.com> References: <20150707144117.5b38ac38efda238af8a1f536@linux-foundation.org> <20150708161815.bdff609d77868dbdc2e1ce64@linux-foundation.org> 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: 3320 Lines: 71 On Tue, Jul 14 2015 at 5:13pm -0400, David Rientjes wrote: > On Thu, 9 Jul 2015, Mikulas Patocka wrote: > > > > > Index: linux-4.2-rc1/mm/util.c > > > > =================================================================== > > > > --- linux-4.2-rc1.orig/mm/util.c 2015-07-07 15:58:11.000000000 +0200 > > > > +++ linux-4.2-rc1/mm/util.c 2015-07-08 19:22:26.000000000 +0200 > > > > @@ -316,6 +316,61 @@ unsigned long vm_mmap(struct file *file, > > > > } > > > > EXPORT_SYMBOL(vm_mmap); > > > > > > > > +void *kvmalloc_node(size_t size, gfp_t gfp, int node) > > > > +{ > > > > + void *p; > > > > + unsigned uninitialized_var(noio_flag); > > > > + > > > > + /* vmalloc doesn't support no-wait allocations */ > > > > + WARN_ON_ONCE(!(gfp & __GFP_WAIT)); > > > > + > > > > + if (likely(size <= KMALLOC_MAX_SIZE)) { > > > > + /* > > > > + * Use __GFP_NORETRY so that we don't loop waiting for the > > > > + * allocation - we don't have to loop here, if the memory > > > > + * is too fragmented, we fallback to vmalloc. > > > > > > I'm not sure about this decision. The direct reclaim retry code is the > > > normal default behaviour and becomes more important with larger allocation > > > attempts. So why turn it off, and make it more likely that we return > > > vmalloc memory? > > > > It can avoid triggering the OOM killer in case of fragmented memory. > > > > This is general question - if the code can handle allocation failure > > gracefully, what gfp flags should it use? Maybe add some flag > > __GFP_MAYFAIL instead of __GFP_NORETRY that changes the behavior in > > desired way? > > > > There's a misunderstanding in regards to the comment: __GFP_NORETRY > doesn't turn direct reclaim or compaction off, it is still attempted and > with the same priority as any other allocation. This only stops the page > allocator from calling the oom killer, which will free memory or panic the > system, and looping when memory is available. > > In regards to the proposal in general, I think it's unnecessary because we > are still left behind with other users who open code their call to > vmalloc. I was interested in commit 058504edd026 ("fs/seq_file: fallback > to vmalloc allocation") since it solved an issue with high memory > fragmentation. Note how it falls back to vmalloc(): _without_ this > __GFP_NORETRY. That's because we only want to fallback when high-order > allocations fail and the page allocator doesn't implicitly loop due to the > order. ext4_kvmalloc(), ext4_kzmalloc() does the same. > > The differences in implementations between those that do kmalloc() and > fallback to vmalloc() are different enough that I don't think we need this > addition. Wouldn't mm benefit from acknowledging the pattern people are open-coding and switching existing code over to official methods for accomplishing the same? 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. -- 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/