Skip to contentSkip to conversation

idr: Rename MAX_LEVEL to MAX_ID_LEVEL

PATCH In review Fengguang Wu 5 participants 9 messages Sep 10, 2012 – Sep 13, 2012

Subsystems (9)
Memory management VFS Kernel janitors Library code +5 more subsystems
lore
Share & tools
lore

Fengguang Wu

author
#0 lore raw

PATCH 1/1 idr: Rename MAX_LEVEL to MAX_ID_LEVEL

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Signed-off-by Fengguang Wu <[email protected]>

---

Andrew: the conflict happens in Glauber's kmemcg-slab tree. So it's better to quickly push this pre-fix to upstream before Glauber's patches.

 include/linux/idr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/idr.h +2 -2
@@ -43,10 +43,10 @@
 #define MAX_ID_MASK (MAX_ID_BIT - 1)
 
 /* Leave the possibility of an incomplete final layer */
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_ID_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
 
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX MAX_ID_LEVEL + MAX_ID_LEVEL
 
 struct idr_layer {
 	unsigned long		 bitmap; /* A zero bit means "space here" */
#2 lore raw
In reply to Fengguang Wu

Am 10.09.2012 15:14, schrieb Fengguang Wu:

Quoted 17 lines

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Signed-off-by Fengguang Wu <[email protected]>

---

Andrew: the conflict happens in Glauber's kmemcg-slab tree. So it's better to quickly push this pre-fix to upstream before Glauber's patches.

 include/linux/idr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/idr.h line 50

Quoted 10 lines
include/linux/idr.h +2 -2
@@ -43,10 +43,10 @@
 #define MAX_ID_MASK (MAX_ID_BIT - 1)
 
 /* Leave the possibility of an incomplete final layer */
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_ID_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
 
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX MAX_ID_LEVEL + MAX_ID_LEVEL

To be fair, i am a bit confused by the naming.
There is MAX_id_LEVEL but idr_BITS are these different things ? If not i would argue to give both the same names either ID or IDR.

re,
wh

Quoted 8 lines
include/linux/idr.h +0 -1
 struct idr_layer {
 	unsigned long		 bitmap; /* A zero bit means "space here" */
--

To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw
In reply to Fengguang Wu

Hi!

Quoted 17 lines · Fengguang Wu

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Signed-off-by Fengguang Wu <[email protected]>

---

Andrew: the conflict happens in Glauber's kmemcg-slab tree. So it's better to quickly push this pre-fix to upstream before Glauber's patches.

 include/linux/idr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/idr.h line 46

Quoted 5 lines
include/linux/idr.h +1 -1
@@ -43,10 +43,10 @@
 #define MAX_ID_MASK (MAX_ID_BIT - 1)
 
 /* Leave the possibility of an incomplete final layer */
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_ID_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS

And while you are it: Please add '(' and ')' around it as in

#define MAX_ID_LEVEL ((MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS)

include/linux/idr.h line 49

Quoted 3 lines
include/linux/idr.h +1 -1
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX MAX_ID_LEVEL + MAX_ID_LEVEL

#define IDR_FREE_MAX (MAX_ID_LEVEL + MAX_ID_LEVEL)

For starters (sleeping in "cpp-101";-): People may use it as in "IDR_FREE_MAX * 2".
And I didn't look into that file - that should be changed everywhere in that way.

Bernd

Signature · 2 lines
Bernd Petrovitsch                  Email : [email protected]
                     LUGA : http://www.luga.at
author
#8 lore raw

Hi Bernd,

Quoted 6 lines

include/linux/idr.h line 46

Quoted 2 lines
include/linux/idr.h +1 -1
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_ID_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS

And while you are it: Please add '(' and ')' around it as in

#define MAX_ID_LEVEL ((MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS)

Good idea. Done.

Quoted 10 lines

include/linux/idr.h line 49

Quoted 4 lines

/* Number of id_layer structs to leave in free list */

include/linux/idr.h +1 -1
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX MAX_ID_LEVEL + MAX_ID_LEVEL

#define IDR_FREE_MAX (MAX_ID_LEVEL + MAX_ID_LEVEL)

For starters (sleeping in "cpp-101";-): People may use it as in "IDR_FREE_MAX * 2".
And I didn't look into that file - that should be changed everywhere in that way.

Sure. It's the only place that need change.

Thanks,
Fengguang

author
#3 lore raw
In reply to walter harms
Quoted 26 lines · walter harms

Am 10.09.2012 15:14, schrieb Fengguang Wu:

Quoted 23 lines

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Signed-off-by Fengguang Wu <[email protected]>

---

Andrew: the conflict happens in Glauber's kmemcg-slab tree. So it's better to quickly push this pre-fix to upstream before Glauber's patches.

 include/linux/idr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/idr.h +1 -1
@@ -43,10 +43,10 @@
 #define MAX_ID_MASK (MAX_ID_BIT - 1)
 
 /* Leave the possibility of an incomplete final layer */
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_ID_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
Quoted 7 lines

include/linux/idr.h line 50

Quoted 4 lines
include/linux/idr.h +1 -1
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX MAX_ID_LEVEL + MAX_ID_LEVEL

To be fair, i am a bit confused by the naming.
There is MAX_id_LEVEL but idr_BITS are these different things ?

Perhaps not. One is derived from the other.

Quoted 1 line

If not i would argue to give both the same names either ID or IDR.

I had the same thought, however gave up at the time because it would make the patch more intrusive. Anyway, here is the new patch for your comments.

--
idr: Rename MAX_LEVEL to MAX_IDR_LEVEL

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

While at it, also make the other names more consistent and add parentheses.

Cc Bernd Petrovitsch <[email protected]>
Cc walter harms <[email protected]>
Signed-off-by Fengguang Wu <[email protected]>
---
 include/linux/idr.h |   10 +++++-----
 lib/idr.c           |   18 +++++++++---------
 2 files changed, 14 insertions(+), 14 deletions(-)
include/linux/idr.h +5 -5
@@ -38,15 +38,15 @@
 #define IDR_SIZE (1 << IDR_BITS)
 #define IDR_MASK ((1 << IDR_BITS)-1)
 
-#define MAX_ID_SHIFT (sizeof(int)*8 - 1)
-#define MAX_ID_BIT (1U << MAX_ID_SHIFT)
-#define MAX_ID_MASK (MAX_ID_BIT - 1)
+#define MAX_IDR_SHIFT (sizeof(int)*8 - 1)
+#define MAX_IDR_BIT (1U << MAX_IDR_SHIFT)
+#define MAX_IDR_MASK (MAX_IDR_BIT - 1)
 
 /* Leave the possibility of an incomplete final layer */
-#define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
+#define MAX_IDR_LEVEL ((MAX_IDR_SHIFT + IDR_BITS - 1) / IDR_BITS)
 
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define MAX_IDR_FREE (MAX_IDR_LEVEL * 2)
 
 struct idr_layer {
 	unsigned long		 bitmap; /* A zero bit means "space here" */
lib/idr.c +9 -9
@@ -20,7 +20,7 @@
  * that id to this code and it returns your pointer.
 
  * You can release ids at any time. When all ids are released, most of
- * the memory is returned (we keep IDR_FREE_MAX) in a local pool so we
+ * the memory is returned (we keep MAX_IDR_FREE) in a local pool so we
  * don't need to go to the memory "store" during an id allocate, just
  * so you don't need to be too concerned about locking and conflicts
  * with the slab allocator.

@@ -122,7 +122,7 @@ static void idr_mark_full(struct idr_lay
  */
 int idr_pre_get(struct idr *idp, gfp_t gfp_mask)
 {
-	while (idp->id_free_cnt < IDR_FREE_MAX) {
+	while (idp->id_free_cnt < MAX_IDR_FREE) {
 		struct idr_layer *new;
 		new = kmem_cache_zalloc(idr_layer_cache, gfp_mask);
 		if (new = NULL)

@@ -179,7 +179,7 @@ static int sub_alloc(struct idr *idp, in
 			sh = IDR_BITS*l;
 			id = ((id >> sh) ^ n ^ m) << sh;
 		}
-		if ((id >= MAX_ID_BIT) || (id < 0))
+		if ((id >= MAX_IDR_BIT) || (id < 0))
 			return IDR_NOMORE_SPACE;
 		if (l = 0)
 			break;

@@ -402,7 +402,7 @@ void idr_remove(struct idr *idp, int id)
 	struct idr_layer *to_free;
 
 	/* Mask off upper bits we don't use for the search. */
-	id &= MAX_ID_MASK;
+	id &= MAX_IDR_MASK;
 
 	sub_remove(idp, (idp->layers - 1) * IDR_BITS, id);
 	if (idp->top && idp->top->count = 1 && (idp->layers > 1) &&

@@ -420,7 +420,7 @@ void idr_remove(struct idr *idp, int id)
 		to_free->bitmap = to_free->count = 0;
 		free_layer(to_free);
 	}
-	while (idp->id_free_cnt >= IDR_FREE_MAX) {
+	while (idp->id_free_cnt >= MAX_IDR_FREE) {
 		p = get_from_free_list(idp);
 		/*
 		 * Note: we don't call the rcu callback here, since the only

@@ -517,7 +517,7 @@ void *idr_find(struct idr *idp, int id)
 	n = (p->layer+1) * IDR_BITS;
 
 	/* Mask off upper bits we don't use for the search. */
-	id &= MAX_ID_MASK;
+	id &= MAX_IDR_MASK;
 
 	if (id >= (1 << n))
 		return NULL;

@@ -659,7 +659,7 @@ void *idr_replace(struct idr *idp, void
 
 	n = (p->layer+1) * IDR_BITS;
 
-	id &= MAX_ID_MASK;
+	id &= MAX_IDR_MASK;
 
 	if (id >= (1 << n))
 		return ERR_PTR(-EINVAL);

@@ -793,7 +793,7 @@ int ida_get_new_above(struct ida *ida, i
 	if (t < 0)
 		return _idr_rc_to_errno(t);
 
-	if (t * IDA_BITMAP_BITS >= MAX_ID_BIT)
+	if (t * IDA_BITMAP_BITS >= MAX_IDR_BIT)
 		return -ENOSPC;
 
 	if (t != idr_id)

@@ -827,7 +827,7 @@ int ida_get_new_above(struct ida *ida, i
 	}
 
 	id = idr_id * IDA_BITMAP_BITS + t;
-	if (id >= MAX_ID_BIT)
+	if (id >= MAX_IDR_BIT)
 		return -ENOSPC;
 
 	__set_bit(t, bitmap->bitmap);
maintainer
#4 lore raw
Quoted 8 lines · On Tue, 11 Sep 2012 17:48:23 +0800 Fengguang Wu <[email protected]> wrote:

idr: Rename MAX_LEVEL to MAX_IDR_LEVEL

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

While at it, also make the other names more consistent and add parentheses.

That was a rather modest effort :(

 drivers/i2c/i2c-core.c        |    2 +-
 drivers/infiniband/core/cm.c  |    2 +-
 drivers/pps/pps.c             |    2 +-
 drivers/thermal/thermal_sys.c |    2 +-
 fs/super.c                    |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
drivers/i2c/i2c-core.c renamed from drivers/i2c/i2c-core.c~idr-rename-max_level-to-max_idr_level-fix +1 -1
diff -puN drivers/i2c/i2c-core.c~idr-rename-max_level-to-max_idr_level-fix drivers/i2c/i2c-core.c
@@ -982,7 +982,7 @@ int i2c_add_numbered_adapter(struct i2c_
 
 	if (adap->nr = -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_ID_MASK)
+	if (adap->nr & ~MAX_IDR_MASK)
 		return -EINVAL;
 
 retry:
drivers/infiniband/core/cm.c renamed from drivers/infiniband/core/cm.c~idr-rename-max_level-to-max_idr_level-fix +1 -1
diff -puN drivers/infiniband/core/cm.c~idr-rename-max_level-to-max_idr_level-fix drivers/infiniband/core/cm.c
@@ -390,7 +390,7 @@ static int cm_alloc_id(struct cm_id_priv
 		ret = idr_get_new_above(&cm.local_id_table, cm_id_priv,
 					next_id, &id);
 		if (!ret)
-			next_id = ((unsigned) id + 1) & MAX_ID_MASK;
+			next_id = ((unsigned) id + 1) & MAX_IDR_MASK;
 		spin_unlock_irqrestore(&cm.lock, flags);
 	} while( (ret = -EAGAIN) && idr_pre_get(&cm.local_id_table, GFP_KERNEL) );
drivers/pps/pps.c renamed from drivers/pps/pps.c~idr-rename-max_level-to-max_idr_level-fix +1 -1
diff -puN drivers/pps/pps.c~idr-rename-max_level-to-max_idr_level-fix drivers/pps/pps.c
@@ -306,7 +306,7 @@ int pps_register_cdev(struct pps_device 
 	if (err < 0)
 		return err;
 
-	pps->id &= MAX_ID_MASK;
+	pps->id &= MAX_IDR_MASK;
 	if (pps->id >= PPS_MAX_SOURCES) {
 		pr_err("%s: too many PPS sources in the system\n",
 					pps->info.name);
drivers/thermal/thermal_sys.c renamed from drivers/thermal/thermal_sys.c~idr-rename-max_level-to-max_idr_level-fix +1 -1
diff -puN drivers/thermal/thermal_sys.c~idr-rename-max_level-to-max_idr_level-fix drivers/thermal/thermal_sys.c
@@ -78,7 +78,7 @@ again:
 	else if (unlikely(err))
 		return err;
 
-	*id = *id & MAX_ID_MASK;
+	*id = *id & MAX_IDR_MASK;
 	return 0;
 }
fs/super.c renamed from fs/super.c~idr-rename-max_level-to-max_idr_level-fix +1 -1
diff -puN fs/super.c~idr-rename-max_level-to-max_idr_level-fix fs/super.c
@@ -871,7 +871,7 @@ int get_anon_bdev(dev_t *p)
 	else if (error)
 		return -EAGAIN;
 
-	if ((dev & MAX_ID_MASK) = (1 << MINORBITS)) {
+	if ((dev & MAX_IDR_MASK) = (1 << MINORBITS)) {
 		spin_lock(&unnamed_dev_lock);
 		ida_remove(&unnamed_dev_ida, dev);
 		if (unnamed_dev_start > dev)

_

author
#5 lore raw
Quoted 21 lines · Andrew Morton
Quoted 8 lines · On Tue, 11 Sep 2012 17:48:23 +0800 Fengguang Wu <[email protected]> wrote:

idr: Rename MAX_LEVEL to MAX_IDR_LEVEL

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

While at it, also make the other names more consistent and add parentheses.

That was a rather modest effort :(

 drivers/i2c/i2c-core.c        |    2 +-
 drivers/infiniband/core/cm.c  |    2 +-
 drivers/pps/pps.c             |    2 +-
 drivers/thermal/thermal_sys.c |    2 +-
 fs/super.c                    |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
Quoted 6 lines

From: Andrew Morton <[email protected]>
Subject: idr-rename-max_level-to-max_idr_level-fix-fix-2

ho hum

 lib/idr.c |   14 +++++++-------

Embarrassing.. Sorry for not build testing it at all!

Regards,
Fengguang

#6 lore raw
Quoted 25 lines · Fengguang Wu
Quoted 24 lines · Andrew Morton
Quoted 8 lines · On Tue, 11 Sep 2012 17:48:23 +0800 Fengguang Wu <[email protected]> wrote:

idr: Rename MAX_LEVEL to MAX_IDR_LEVEL

To avoid name conflicts:

drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

While at it, also make the other names more consistent and add parentheses.

That was a rather modest effort :(

 drivers/i2c/i2c-core.c        |    2 +-
 drivers/infiniband/core/cm.c  |    2 +-
 drivers/pps/pps.c             |    2 +-
 drivers/thermal/thermal_sys.c |    2 +-
 fs/super.c                    |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

From: Andrew Morton <[email protected]>
Subject: idr-rename-max_level-to-max_idr_level-fix-fix-2

Quoted 9 lines
Quoted 3 lines

ho hum

 lib/idr.c |   14 +++++++-------

Embarrassing.. Sorry for not build testing it at all!

Regards,
Fengguang

You can build test it automatically using Fengguang's 0-day test system.

/me runs