From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754434AbbCaVUb (ORCPT ); Tue, 31 Mar 2015 17:20:31 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:54146 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754406AbbCaVU0 (ORCPT ); Tue, 31 Mar 2015 17:20:26 -0400 Date: Tue, 31 Mar 2015 14:20:25 -0700 From: Andrew Morton To: Christoph Lameter Cc: Jesper Dangaard Brouer , linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linuxfoundation.org, Pekka Enberg , iamjoonsoo@lge.com Subject: Re: Slab infrastructure for bulk object allocation and freeing V2 Message-Id: <20150331142025.63249f2f0189aee231a6e0c8@linux-foundation.org> In-Reply-To: References: X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 30 Mar 2015 09:31:19 -0500 (CDT) Christoph Lameter wrote: > After all of the earlier discussions I thought it would be better to > first get agreement on the basic way to allow implementation of the > bulk alloc in the common slab code. So this is a revision of the initial > proposal and it just covers the first patch. > > > > This patch adds the basic infrastructure for alloc / free operations > on pointer arrays. It includes a generic function in the common > slab code that is used in this infrastructure patch to > create the unoptimized functionality for slab bulk operations. > > Allocators can then provide optimized allocation functions > for situations in which large numbers of objects are needed. > These optimization may avoid taking locks repeatedly and > bypass metadata creation if all objects in slab pages > can be used to provide the objects required. This patch doesn't really do anything. I guess nailing down the interface helps a bit. > @@ -289,6 +289,8 @@ static __always_inline int kmalloc_index > void *__kmalloc(size_t size, gfp_t flags); > void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags); > void kmem_cache_free(struct kmem_cache *, void *); > +void kmem_cache_free_array(struct kmem_cache *, size_t, void **); > +int kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **); > > #ifdef CONFIG_NUMA > void *__kmalloc_node(size_t size, gfp_t flags, int node); > Index: linux/mm/slab_common.c > =================================================================== > --- linux.orig/mm/slab_common.c 2015-03-30 08:48:12.923927793 -0500 > +++ linux/mm/slab_common.c 2015-03-30 08:57:41.737572817 -0500 > @@ -105,6 +105,29 @@ static inline int kmem_cache_sanity_chec > } > #endif > > +int __kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr, > + void **p) > +{ > + size_t i; > + > + for (i = 0; i < nr; i++) { > + void *x = p[i] = kmem_cache_alloc(s, flags); > + if (!x) > + return i; > + } > + return nr; > +} Some documentation would be nice. It's a major new interface, exported to modules. And it isn't completely obvious, because the return semantics are weird. What's the reason for returning a partial result when ENOMEM? Some callers will throw away the partial result and simply fail out. If a caller attempts to go ahead and use the partial result then great, but you can bet that nobody will actually runtime test this situation, so the interface is an invitation for us to release partially-tested code into the wild. Instead of the above, did you consider doing int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr, ? This way we save a level of function call and all that wrapper code in the allocators simply disappears. > --- linux.orig/mm/slab.c 2015-03-30 08:48:12.923927793 -0500 > +++ linux/mm/slab.c 2015-03-30 08:49:08.398137844 -0500 > @@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache > } > EXPORT_SYMBOL(kmem_cache_alloc); > > +void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) { > + __kmem_cache_free_array(s, size, p); > +} Coding style is weird.