All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: akpm@linux-foundation.org
Cc: containers@lists.linux-foundation.org, bblum@google.com,
	linux-kernel@vger.kernel.org, menage@google.com,
	vda.linux@googlemail.com, mikew@google.com,
	Dave Hansen <dave@linux.vnet.ibm.com>
Subject: [RFCv2][PATCH] flexible array implementation
Date: Tue, 21 Jul 2009 15:00:17 -0700	[thread overview]
Message-ID: <20090721220017.60A219D3@kernel> (raw)



Changes from v1:
- to vs too typo
- added __check_part_and_nr() and gave it a warning
- fixed off-by-one check on __nr_part_ptrs()
- addedFLEX_ARRAY_INIT() macro
- some kerneldoc comments about the capacity
  with various sized objects
- comments to note lack of locking semantice

--

Once a structure goes over PAGE_SIZE*2, we see occasional
allocation failures.  Some people have chosen to switch
over to things like vmalloc() that will let them keep
array-like access to such a large structures.  But,
vmalloc() has plenty of downsides.

Here's an alternative.  I think it's what Andrew was
suggesting  here:

	http://lkml.org/lkml/2009/7/2/518 

I call it a flexible array.  It does all of its work in
PAGE_SIZE bits, so never does an order>0 allocation.
The base level has PAGE_SIZE-2*sizeof(int) bytes of
storage for pointers to the second level.  So, with a
32-bit arch, you get about 4MB (4183112 bytes) of total
storage when the objects pack nicely into a page.  It
is half that on 64-bit because the pointers are twice
the size.

The interface is dirt simple.  4 functions:
	alloc_flex_array()
	free_flex_array()
	flex_array_put()
	flex_array_get()

put() appends an item into the array while get() takes
indexes and does array-style access.

One thought is that we should perhaps make the base
structure half the size on 32-bit arches.  That will
ensure that someone testing on 32-bit will not get
bitten by the size shrinking by half when moving to
64-bit.

We could also potentially just pass the "element_size"
into each of the API functions instead of storing it
internally.  That would get us one more base pointer
on 32-bit.

The last improvement that I thought about was letting
the individual array members span pages.  In this
implementation, if you have a 2049-byte object, it
will only pack one of them into each "part" with
no attempt to pack them.  At this point, I don't think
the added complexity would be worth it.

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/include/linux/flex_array.h |   45 +++++
 linux-2.6.git-dave/lib/Makefile               |    2 
 linux-2.6.git-dave/lib/flex_array.c           |  230 ++++++++++++++++++++++++++
 3 files changed, 276 insertions(+), 1 deletion(-)

diff -puN /dev/null include/linux/flex_array.h
--- /dev/null	2008-09-02 09:40:19.000000000 -0700
+++ linux-2.6.git-dave/include/linux/flex_array.h	2009-07-21 14:55:35.000000000 -0700
@@ -0,0 +1,45 @@
+#ifndef _FLEX_ARRAY_H
+#define _FLEX_ARRAY_H
+
+#include <linux/types.h>
+#include <asm/page.h>
+
+#define FLEX_ARRAY_PART_SIZE PAGE_SIZE
+#define FLEX_ARRAY_BASE_SIZE PAGE_SIZE
+
+struct flex_array_part;
+
+/*
+ * This is meant too replace cases where an array-like
+ * structure has gotten to big to fit into kmalloc()
+ * and the developer is getting tempted to use
+ * vmalloc().
+ */
+
+struct flex_array {
+	union {
+		struct {
+			int nr_elements;
+			int element_size;
+			struct flex_array_part *parts[0];
+		};
+		/*
+		 * This little trick makes sure that
+		 * sizeof(flex_array) == PAGE_SIZE
+		 */
+		char padding[FLEX_ARRAY_BASE_SIZE];
+	};
+};
+
+#define FLEX_ARRAY_INIT(size, total) {{{\
+	.element_size = (size),		\
+	.nr_elements = 0,		\
+}}}
+
+struct flex_array *flex_array_alloc(int element_size, int total, gfp_t flags);
+void flex_array_free(struct flex_array *fa);
+int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags);
+int flex_array_append(struct flex_array *fa, void *src, gfp_t flags);
+void *flex_array_get(struct flex_array *fa, int element_nr);
+
+#endif /* _FLEX_ARRAY_H */
diff -puN /dev/null lib/flex_array.c
--- /dev/null	2008-09-02 09:40:19.000000000 -0700
+++ linux-2.6.git-dave/lib/flex_array.c	2009-07-21 14:52:09.000000000 -0700
@@ -0,0 +1,230 @@
+/*
+ * Flexible array managed in PAGE_SIZE parts
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright IBM Corporation, 2009
+ *
+ * Author: Dave Hansen <dave@linux.vnet.ibm.com>
+ */
+
+#include <linux/flex_array.h>
+#include <linux/slab.h>
+#include <linux/stddef.h>
+
+struct flex_array_part {
+	char elements[FLEX_ARRAY_PART_SIZE];
+};
+
+static inline int __elements_per_part(int element_size)
+{
+	return FLEX_ARRAY_PART_SIZE / element_size;
+}
+
+static inline int __nr_part_ptrs(void)
+{
+	int element_offset = offsetof(struct flex_array, parts);
+	int bytes_left = FLEX_ARRAY_BASE_SIZE - element_offset;
+	return bytes_left / sizeof(struct flex_array_part *);
+}
+
+/**
+ * flex_array_alloc - allocate a new flexible array
+ * @element_size:	the size of individual elements in the array
+ * @total:		total number of elements that this should hold
+ *
+ * Note: all locking must be provided by the caller.
+ *
+ * We do not actually use @total to size the allocation at this
+ * point.  It is just used to ensure that the user does not try
+ * to use this structure for something larger than it can handle
+ * later on.
+ *
+ * The maximum number of elements is defined as: the number of
+ * elements that can be stored in a page times the number of
+ * page pointers that we can fit in the base structure or (using
+ * integer math):
+ *
+ * 	(PAGE_SIZE/element_size) * (PAGE_SIZE-8)/sizeof(void *)
+ *
+ * Here's a table showing example capacities.  Note that the maximum
+ * index that the get/put() functions is just nr_objects-1.
+ *
+ * Element size | Objects  | Objects |
+ * PAGE_SIZE=4k |  32-bit  |  64-bit |
+ * ----------------------------------|
+ *      1 byte  |  4186112 | 2093056 |
+ *      2 bytes |  2093056 | 1046528 |
+ *      3 bytes |  1395030 |  697515 |
+ *      4 bytes |  1046528 |  523264 |
+ *     32 bytes |   130816 |   65408 |
+ *     33 bytes |   126728 |   63364 |
+ *   2048 bytes |     2044 |   10228 |
+ *   2049 bytes |     1022 |     511 |
+ *       void * |  1046528 |  261632 |
+ *
+ * Since 64-bit pointers are twice the size, we lose half the
+ * capacity in the base structure.  Also note that no effort is made
+ * to efficiently pack objects across page boundaries.
+ */
+struct flex_array *flex_array_alloc(int element_size, int total, gfp_t flags)
+{
+	struct flex_array *ret;
+	int max_size = __nr_part_ptrs() * __elements_per_part(element_size);
+
+	/* max_size will end up 0 if element_size > PAGE_SIZE */
+	if (total > max_size)
+		return NULL;
+	ret = kzalloc(sizeof(struct flex_array), flags);
+	if (!ret)
+		return NULL;
+	ret->element_size = element_size;
+	return ret;
+}
+
+static int fa_element_to_part_nr(struct flex_array *fa, int element_nr)
+{
+	return element_nr / __elements_per_part(fa->element_size);
+}
+
+void flex_array_free(struct flex_array *fa)
+{
+	int part_nr;
+	int max_part;
+
+	/* keeps us from getting the index of -1 below */
+	if (!fa->nr_elements)
+		goto free_base;
+
+	/* we really want the *index* of the last element, thus the -1 */
+	max_part = fa_element_to_part_nr(fa, fa->nr_elements-1);
+	for (part_nr = 0; part_nr <= max_part; part_nr++)
+		kfree(fa->parts[part_nr]);
+free_base:
+	kfree(fa);
+}
+
+static int fa_index_inside_part(struct flex_array *fa, int element_nr)
+{
+	return (element_nr % __elements_per_part(fa->element_size));
+}
+
+static int offset_inside_part(struct flex_array *fa, int element_nr)
+{
+	int part_offset = fa_index_inside_part(fa, element_nr);
+	return part_offset * fa->element_size;
+}
+
+static int __check_part_and_nr(struct flex_array *fa,
+	   		       int part_nr, int element_nr)
+{
+	if (part_nr >= __nr_part_ptrs() ||
+	    element_nr > fa->nr_elements) {
+		WARN(1, "bad flexible array element number: %d > %d\n",
+			element_nr, fa->nr_elements);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct flex_array_part *
+__fa_get_part(struct flex_array *fa, int part_nr, gfp_t flags)
+{
+	struct flex_array_part *part = NULL;
+	if (__check_part_and_nr(fa, part_nr, fa->nr_elements))
+		return NULL;
+	part = fa->parts[part_nr];
+	if (!part) {
+		part = kmalloc(FLEX_ARRAY_PART_SIZE, flags);
+		if (!part)
+			return NULL;
+		fa->parts[part_nr] = part;
+	}
+	return part;
+}
+
+/**
+ * flex_array_put - copy data into the array at @element_nr
+ * @src:	address of data to copy into the array
+ * @element_nr:	index of the position in which to insert
+ * 		the new element.
+ *
+ * Note that this *copies* the contents of @src into
+ * the array.  If you are trying to store an array of
+ * pointers, make sure to pass in &ptr instead of ptr.
+ *
+ * Locking must be provided by the caller.
+ */
+int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags)
+{
+	int part_nr = fa_element_to_part_nr(fa, element_nr);
+	struct flex_array_part *part;
+	void *dst;
+
+	part = __fa_get_part(fa, part_nr, flags);
+	if (!part)
+		return -ENOMEM;
+	dst = &part->elements[offset_inside_part(fa, element_nr)];
+	memcpy(dst, src, fa->element_size);
+	return 0;
+}
+
+/**
+ * flex_array_append - append a new member into the array
+ * @src:	address of data to copy into the array
+ *
+ * This will use the internally-remembered last position in
+ * the array to choose an insertion point.
+ *
+ * Note that this *copies* the contents of @src into
+ * the array.  If you are trying to store an array of
+ * pointers, make sure to pass in &ptr instead of ptr.
+ *
+ * Locking must be provided by the caller.
+ */
+int flex_array_append(struct flex_array *fa, void *src, gfp_t flags)
+{
+	int ret = flex_array_put(fa, fa->nr_elements, src, flags);
+	if (!ret)
+		fa->nr_elements++;
+	return ret;
+}
+
+
+/**
+ * flex_array_get - pull data back out of the array
+ * @element_nr:	index of the element to fetch from the array
+ *
+ * Returns a pointer to the data at index @element_nr.  Note
+ * that this is a copy of the data that was passed in.  If you
+ * are using this to store pointers, you'll get back &ptr.
+ *
+ * Locking must be provided by the caller.
+ */
+void *flex_array_get(struct flex_array *fa, int element_nr)
+{
+	int part_nr = fa_element_to_part_nr(fa, element_nr);
+	struct flex_array_part *part;
+	int offset;
+
+	if (__check_part_and_nr(fa, part_nr, fa->nr_elements))
+		return NULL;
+ 	if (!fa->parts[part_nr])
+		return NULL;
+
+	part = fa->parts[part_nr];
+	offset = offset_inside_part(fa, element_nr);
+	return &part->elements[offset_inside_part(fa, element_nr)];
+}
diff -puN lib/Makefile~fa lib/Makefile
--- linux-2.6.git/lib/Makefile~fa	2009-07-21 14:42:37.000000000 -0700
+++ linux-2.6.git-dave/lib/Makefile	2009-07-21 14:42:37.000000000 -0700
@@ -12,7 +12,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
 	 idr.o int_sqrt.o extable.o prio_tree.o \
 	 sha1.o irq_regs.o reciprocal_div.o argv_split.o \
 	 proportions.o prio_heap.o ratelimit.o show_mem.o \
-	 is_single_threaded.o plist.o decompress.o
+	 is_single_threaded.o plist.o decompress.o flex_array.o
 
 lib-$(CONFIG_MMU) += ioremap.o
 lib-$(CONFIG_SMP) += cpumask.o
diff -puN lib/radix-tree.c~fa lib/radix-tree.c
diff -puN ./include/linux/radix-tree.h~fa ./include/linux/radix-tree.h
_

             reply	other threads:[~2009-07-21 22:00 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-21 22:00 Dave Hansen [this message]
2009-07-21 22:09 ` [RFCv2][PATCH] flexible array implementation Dave Hansen
2009-07-21 22:09 ` Dave Hansen
2009-07-21 22:35   ` Andrew Morton
2009-07-21 22:35   ` Andrew Morton
2009-07-22  3:25 ` Li Zefan
2009-07-22  4:34   ` Dave Hansen
2009-07-22  6:14     ` Li Zefan
2009-07-22  6:14     ` Li Zefan
     [not found]   ` <4A66868F.4010001-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2009-07-22  4:34     ` Dave Hansen
2009-07-22  3:25 ` Li Zefan
2009-07-22  7:09 ` Amerigo Wang
2009-07-22  7:09 ` Amerigo Wang
     [not found]   ` <20090722070903.GG6281-+dguKlz9DXUf7BdofF/totBPR1lH4CV8@public.gmane.org>
2009-07-22 15:02     ` Dave Hansen
2009-07-22 15:02   ` Dave Hansen
2009-07-22 18:30 ` Matt Helsley
2009-07-22 22:03   ` Dave Hansen
     [not found]   ` <20090722183018.GE5878-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>
2009-07-22 22:03     ` Dave Hansen
2009-07-22 18:30 ` Matt Helsley
2009-07-22 19:55 ` Mike Waychison
2009-07-22 19:55   ` Mike Waychison
     [not found]   ` <4A676ECC.1090400-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2009-07-22 22:00     ` Dave Hansen
2009-07-22 22:00   ` Dave Hansen
2009-07-22 20:57 ` Benjamin Blum
2009-07-22 20:57 ` Benjamin Blum
     [not found]   ` <2f86c2480907221357j5a09fd9au890f815db3750187-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-07-22 21:51     ` Dave Hansen
2009-07-22 21:51       ` Dave Hansen
2009-07-22 23:20       ` Benjamin Blum
2009-07-22 23:20       ` Benjamin Blum
2009-07-23  5:41         ` Dave Hansen
     [not found]         ` <2f86c2480907221620u6da3a8e3h8273f6a9ee156f86-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-07-23  5:41           ` Dave Hansen
2009-07-23  2:45 ` H. Peter Anvin
     [not found]   ` <4A67CEB7.2080505-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2009-07-23 14:30     ` Dave Hansen
2009-07-23 14:30       ` Dave Hansen
2009-07-23  2:45 ` H. Peter Anvin
  -- strict thread matches above, loose matches on Subject: below --
2009-07-21 22:00 Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090721220017.60A219D3@kernel \
    --to=dave@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=bblum@google.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=menage@google.com \
    --cc=mikew@google.com \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.