From: Corey Minyard <minyard@acm.org>
There were definately some problems in there. I've made some changes and
tested with a lot of bounds. I don't have a machine with enough memory to
fill it up (it would take ~16GB on a 64-bit machine), but I use the "above"
code to simulate a lot of situations.
The problems were:
* IDR_FULL was not the right value
* idr_get_new_above() was not defined in the headers or documented.
* idr_alloc() bug-ed if there was a race and not enough memory was
allocated. It should have returned NULL.
* id will overflow when you go past the end.
* There was a "(id >= (1 << (layers*IDR_BITS)))" comparison, but at
the top layer it would overflow the id and be zero.
* The allocation should return ENOSPC for an "above" value with
nothing above it, but it returned EAGAIN.
I have not tested on 64-bits (as I don't have a 64-bit machine).
I've included the files, a diff from the previous version, and my test
programs.
For the test programs, idr_test <size> will just attempt to allocate
<size> elements, check them, free them, and check them again.
idr_test2 <size> <incr> will allocate <size> element with <incr> between
them.
idr_test3 just tests some bounds and tries all values with just a few in
the idr.
---
25-akpm/include/linux/idr.h | 15 ++++++++------
25-akpm/lib/idr.c | 45 ++++++++++++++++++++++++++++++--------------
2 files changed, 40 insertions(+), 20 deletions(-)
diff -puN include/linux/idr.h~idr-fixups include/linux/idr.h
--- 25/include/linux/idr.h~idr-fixups 2004-05-13 17:05:16.927944568 -0700
+++ 25-akpm/include/linux/idr.h 2004-05-13 17:05:16.932943808 -0700
@@ -14,15 +14,17 @@
#if BITS_PER_LONG == 32
# define IDR_BITS 5
# define IDR_FULL 0xfffffffful
-/* We can only use half the bits in the top level because there are
- only four possible bits in the top level (5 bits * 4 levels = 25
- bits, but you only use 24 bits in the id). */
-# define TOP_LEVEL_FULL (IDR_FULL >> 16)
+/* We can only use two of the bits in the top level because there is
+ only one possible bit in the top level (5 bits * 7 levels = 35
+ bits, but you only use 31 bits in the id). */
+# define TOP_LEVEL_FULL (IDR_FULL >> 30)
#elif BITS_PER_LONG == 64
# define IDR_BITS 6
# define IDR_FULL 0xfffffffffffffffful
-/* We can use all the bits in a 64-bit long at the top level. */
-# define TOP_LEVEL_FULL IDR_FULL
+/* We can only use two of the bits in the top level because there is
+ only one possible bit in the top level (6 bits * 6 levels = 36
+ bits, but you only use 31 bits in the id). */
+# define TOP_LEVEL_FULL (IDR_FULL >> 62)
#else
# error "BITS_PER_LONG is not 32 or 64"
#endif
@@ -71,6 +73,7 @@ struct idr {
void *idr_find(struct idr *idp, int id);
int idr_pre_get(struct idr *idp, unsigned gfp_mask);
int idr_get_new(struct idr *idp, void *ptr, int *id);
+int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id);
void idr_remove(struct idr *idp, int id);
void idr_init(struct idr *idp);
diff -puN lib/idr.c~idr-fixups lib/idr.c
--- 25/lib/idr.c~idr-fixups 2004-05-13 17:05:16.929944264 -0700
+++ 25-akpm/lib/idr.c 2004-05-13 17:05:16.933943656 -0700
@@ -72,6 +72,11 @@
* with the id. The value is returned in the "id" field. idr_get_new()
* returns a value in the range 0 ... 0x7fffffff
+ * int idr_get_new_above(struct idr *idp, void *ptr, int start_id, int *id);
+
+ * Like idr_get_new(), but the returned id is guaranteed to be at or
+ * above start_id.
+
* void *idr_find(struct idr *idp, int id);
* returns the "ptr", given the id. A NULL return indicates that the
@@ -112,7 +117,7 @@ static struct idr_layer *alloc_layer(str
spin_lock(&idp->lock);
if (!(p = idp->id_free))
- BUG();
+ return NULL;
idp->id_free = p->ary[0];
idp->id_free_cnt--;
p->ary[0] = 0;
@@ -178,8 +183,8 @@ static int sub_alloc(struct idr *idp, vo
sh = IDR_BITS*l;
id = ((id >> sh) ^ n ^ m) << sh;
}
- if (id >= MAX_ID_BIT)
- return -1;
+ if ((id >= MAX_ID_BIT) || (id < 0))
+ return -3;
if (l == 0)
break;
/*
@@ -217,7 +222,7 @@ static int sub_alloc(struct idr *idp, vo
return(id);
}
-int idr_get_new_above(struct idr *idp, void *ptr, int starting_id)
+static int idr_get_new_above_int(struct idr *idp, void *ptr, int starting_id)
{
struct idr_layer *p, *new;
int layers, v, id;
@@ -235,7 +240,7 @@ build_up:
* Add a new layer to the top of the tree if the requested
* id is larger than the currently allocated space.
*/
- while (id >= (1 << (layers*IDR_BITS))) {
+ while ((layers < MAX_LEVEL) && (id >= (1 << (layers*IDR_BITS)))) {
layers++;
if (!p->count)
continue;
@@ -265,27 +270,39 @@ build_up:
goto build_up;
return(v);
}
-EXPORT_SYMBOL(idr_get_new_above);
-static int idr_full(struct idr *idp)
+int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id)
{
- return ((idp->layers >= MAX_LEVEL)
- && (idp->top->bitmap == TOP_LEVEL_FULL));
+ int rv;
+ rv = idr_get_new_above_int(idp, ptr, starting_id);
+ /*
+ * This is a cheap hack until the IDR code can be fixed to
+ * return proper error values.
+ */
+ if (rv < 0) {
+ if (rv == -1)
+ return -EAGAIN;
+ else /* Will be -3 */
+ return -ENOSPC;
+ }
+ *id = rv;
+ return 0;
}
+EXPORT_SYMBOL(idr_get_new_above);
int idr_get_new(struct idr *idp, void *ptr, int *id)
{
int rv;
- rv = idr_get_new_above(idp, ptr, 0);
+ rv = idr_get_new_above_int(idp, ptr, 0);
/*
* This is a cheap hack until the IDR code can be fixed to
* return proper error values.
*/
- if (rv == -1) {
- if (idr_full(idp))
- return -ENOSPC;
- else
+ if (rv < 0) {
+ if (rv == -1)
return -EAGAIN;
+ else /* Will be -3 */
+ return -ENOSPC;
}
*id = rv;
return 0;
_