From: Corey Minyard 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 will just attempt to allocate elements, check them, free them, and check them again. idr_test2 will allocate element with 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 0xffffffff -/* 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 0xffffffffffffffff -/* 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; _