aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTejun Heo <tj@kernel.org>2013-02-27 20:05:04 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2013-02-27 22:10:20 -0500
commite8c8d1bc063bc88cfa1356266027b5075d3a82d7 (patch)
treeb657e3a3aa9eb8499690bb2fc0cbd908cdd2a334
parent326cf0f0f308933c10236280a322031f0097205d (diff)
idr: remove MAX_IDR_MASK and move left MAX_IDR_* into idr.c
MAX_IDR_MASK is another weirdness in the idr interface. As idr covers whole positive integer range, it's defined as 0x7fffffff or INT_MAX. Its usage in idr_find(), idr_replace() and idr_remove() is bizarre. They basically mask off the sign bit and operate on the rest, so if the caller, by accident, passes in a negative number, the sign bit will be masked off and the remaining part will be used as if that was the input, which is worse than crashing. The constant is visible in idr.h and there are several users in the kernel. * drivers/i2c/i2c-core.c:i2c_add_numbered_adapter() Basically used to test if adap->nr is a negative number which isn't -1 and returns -EINVAL if so. idr_alloc() already has negative @start checking (w/ WARN_ON_ONCE), so this can go away. * drivers/infiniband/core/cm.c:cm_alloc_id() drivers/infiniband/hw/mlx4/cm.c:id_map_alloc() Used to wrap cyclic @start. Can be replaced with max(next, 0). Note that this type of cyclic allocation using idr is buggy. These are prone to spurious -ENOSPC failure after the first wraparound. * fs/super.c:get_anon_bdev() The ID allocated from ida is masked off before being tested whether it's inside valid range. ida allocated ID can never be a negative number and the masking is unnecessary. Update idr_*() functions to fail with -EINVAL when negative @id is specified and update other MAX_IDR_MASK users as described above. This leaves MAX_IDR_MASK without any user, remove it and relocate other MAX_IDR_* constants to lib/idr.c. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Jean Delvare <khali@linux-fr.org> Cc: Roland Dreier <roland@kernel.org> Cc: Sean Hefty <sean.hefty@intel.com> Cc: Hal Rosenstock <hal.rosenstock@gmail.com> Cc: "Marciniszyn, Mike" <mike.marciniszyn@intel.com> Cc: Jack Morgenstein <jackm@dev.mellanox.co.il> Cc: Or Gerlitz <ogerlitz@mellanox.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Acked-by: Wolfram Sang <wolfram@the-dreams.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--drivers/i2c/i2c-core.c2
-rw-r--r--drivers/infiniband/core/cm.c2
-rw-r--r--drivers/infiniband/hw/mlx4/cm.c2
-rw-r--r--fs/super.c2
-rw-r--r--include/linux/idr.h10
-rw-r--r--lib/idr.c24
6 files changed, 20 insertions, 22 deletions
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 8d1f644a7fdc..991d38daa87d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -979,8 +979,6 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adap)
979 979
980 if (adap->nr == -1) /* -1 means dynamically assign bus id */ 980 if (adap->nr == -1) /* -1 means dynamically assign bus id */
981 return i2c_add_adapter(adap); 981 return i2c_add_adapter(adap);
982 if (adap->nr & ~MAX_IDR_MASK)
983 return -EINVAL;
984 982
985 mutex_lock(&core_lock); 983 mutex_lock(&core_lock);
986 id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1, 984 id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1,
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 98281fe5ea4b..784b97cb05b0 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -390,7 +390,7 @@ static int cm_alloc_id(struct cm_id_private *cm_id_priv)
390 390
391 id = idr_alloc(&cm.local_id_table, cm_id_priv, next_id, 0, GFP_NOWAIT); 391 id = idr_alloc(&cm.local_id_table, cm_id_priv, next_id, 0, GFP_NOWAIT);
392 if (id >= 0) 392 if (id >= 0)
393 next_id = ((unsigned) id + 1) & MAX_IDR_MASK; 393 next_id = max(id + 1, 0);
394 394
395 spin_unlock_irqrestore(&cm.lock, flags); 395 spin_unlock_irqrestore(&cm.lock, flags);
396 idr_preload_end(); 396 idr_preload_end();
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index 80e59ed864b3..e0d79b2395e4 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -225,7 +225,7 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
225 225
226 ret = idr_alloc(&sriov->pv_id_table, ent, next_id, 0, GFP_NOWAIT); 226 ret = idr_alloc(&sriov->pv_id_table, ent, next_id, 0, GFP_NOWAIT);
227 if (ret >= 0) { 227 if (ret >= 0) {
228 next_id = ((unsigned)ret + 1) & MAX_IDR_MASK; 228 next_id = max(ret + 1, 0);
229 ent->pv_cm_id = (u32)ret; 229 ent->pv_cm_id = (u32)ret;
230 sl_id_map_add(ibdev, ent); 230 sl_id_map_add(ibdev, ent);
231 list_add_tail(&ent->list, &sriov->cm_list); 231 list_add_tail(&ent->list, &sriov->cm_list);
diff --git a/fs/super.c b/fs/super.c
index 12f123712161..df6c2f4c6b59 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -842,7 +842,7 @@ int get_anon_bdev(dev_t *p)
842 else if (error) 842 else if (error)
843 return -EAGAIN; 843 return -EAGAIN;
844 844
845 if ((dev & MAX_IDR_MASK) == (1 << MINORBITS)) { 845 if (dev == (1 << MINORBITS)) {
846 spin_lock(&unnamed_dev_lock); 846 spin_lock(&unnamed_dev_lock);
847 ida_remove(&unnamed_dev_ida, dev); 847 ida_remove(&unnamed_dev_ida, dev);
848 if (unnamed_dev_start > dev) 848 if (unnamed_dev_start > dev)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 6dcf133f208a..99b0ce533f0e 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -38,16 +38,6 @@
38#define IDR_SIZE (1 << IDR_BITS) 38#define IDR_SIZE (1 << IDR_BITS)
39#define IDR_MASK ((1 << IDR_BITS)-1) 39#define IDR_MASK ((1 << IDR_BITS)-1)
40 40
41#define MAX_IDR_SHIFT (sizeof(int)*8 - 1)
42#define MAX_IDR_BIT (1U << MAX_IDR_SHIFT)
43#define MAX_IDR_MASK (MAX_IDR_BIT - 1)
44
45/* Leave the possibility of an incomplete final layer */
46#define MAX_IDR_LEVEL ((MAX_IDR_SHIFT + IDR_BITS - 1) / IDR_BITS)
47
48/* Number of id_layer structs to leave in free list */
49#define MAX_IDR_FREE (MAX_IDR_LEVEL * 2)
50
51struct idr_layer { 41struct idr_layer {
52 unsigned long bitmap; /* A zero bit means "space here" */ 42 unsigned long bitmap; /* A zero bit means "space here" */
53 struct idr_layer __rcu *ary[1<<IDR_BITS]; 43 struct idr_layer __rcu *ary[1<<IDR_BITS];
diff --git a/lib/idr.c b/lib/idr.c
index 63dda62131b3..e2b799989ab0 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -38,6 +38,15 @@
38#include <linux/percpu.h> 38#include <linux/percpu.h>
39#include <linux/hardirq.h> 39#include <linux/hardirq.h>
40 40
41#define MAX_IDR_SHIFT (sizeof(int) * 8 - 1)
42#define MAX_IDR_BIT (1U << MAX_IDR_SHIFT)
43
44/* Leave the possibility of an incomplete final layer */
45#define MAX_IDR_LEVEL ((MAX_IDR_SHIFT + IDR_BITS - 1) / IDR_BITS)
46
47/* Number of id_layer structs to leave in free list */
48#define MAX_IDR_FREE (MAX_IDR_LEVEL * 2)
49
41static struct kmem_cache *idr_layer_cache; 50static struct kmem_cache *idr_layer_cache;
42static DEFINE_PER_CPU(struct idr_layer *, idr_preload_head); 51static DEFINE_PER_CPU(struct idr_layer *, idr_preload_head);
43static DEFINE_PER_CPU(int, idr_preload_cnt); 52static DEFINE_PER_CPU(int, idr_preload_cnt);
@@ -542,8 +551,8 @@ void idr_remove(struct idr *idp, int id)
542 struct idr_layer *p; 551 struct idr_layer *p;
543 struct idr_layer *to_free; 552 struct idr_layer *to_free;
544 553
545 /* Mask off upper bits we don't use for the search. */ 554 if (WARN_ON_ONCE(id < 0))
546 id &= MAX_IDR_MASK; 555 return;
547 556
548 sub_remove(idp, (idp->layers - 1) * IDR_BITS, id); 557 sub_remove(idp, (idp->layers - 1) * IDR_BITS, id);
549 if (idp->top && idp->top->count == 1 && (idp->layers > 1) && 558 if (idp->top && idp->top->count == 1 && (idp->layers > 1) &&
@@ -650,14 +659,14 @@ void *idr_find(struct idr *idp, int id)
650 int n; 659 int n;
651 struct idr_layer *p; 660 struct idr_layer *p;
652 661
662 if (WARN_ON_ONCE(id < 0))
663 return NULL;
664
653 p = rcu_dereference_raw(idp->top); 665 p = rcu_dereference_raw(idp->top);
654 if (!p) 666 if (!p)
655 return NULL; 667 return NULL;
656 n = (p->layer+1) * IDR_BITS; 668 n = (p->layer+1) * IDR_BITS;
657 669
658 /* Mask off upper bits we don't use for the search. */
659 id &= MAX_IDR_MASK;
660
661 if (id > idr_max(p->layer + 1)) 670 if (id > idr_max(p->layer + 1))
662 return NULL; 671 return NULL;
663 BUG_ON(n == 0); 672 BUG_ON(n == 0);
@@ -799,14 +808,15 @@ void *idr_replace(struct idr *idp, void *ptr, int id)
799 int n; 808 int n;
800 struct idr_layer *p, *old_p; 809 struct idr_layer *p, *old_p;
801 810
811 if (WARN_ON_ONCE(id < 0))
812 return ERR_PTR(-EINVAL);
813
802 p = idp->top; 814 p = idp->top;
803 if (!p) 815 if (!p)
804 return ERR_PTR(-EINVAL); 816 return ERR_PTR(-EINVAL);
805 817
806 n = (p->layer+1) * IDR_BITS; 818 n = (p->layer+1) * IDR_BITS;
807 819
808 id &= MAX_IDR_MASK;
809
810 if (id >= (1 << n)) 820 if (id >= (1 << n))
811 return ERR_PTR(-EINVAL); 821 return ERR_PTR(-EINVAL);
812 822