linux-kernel - Re: [PATCH 2/7] mm: introduce kvmalloc and kvmalloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Jul 2015 14:13:16 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Mikulas Patocka <mpatocka@...hat.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Snitzer <msnitzer@...hat.com>,
	"Alasdair G. Kergon" <agk@...hat.com>,
	Edward Thornber <thornber@...hat.com>,
	Vivek Goyal <vgoyal@...hat.com>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, dm-devel@...hat.com,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 2/7] mm: introduce kvmalloc and kvmalloc_node

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/