Skip to contentSkip to conversation

mm/memcontrol.c:755:6: sparse: symbol 'memcg_flush_cache_create_qu

PATCH In review Fengguang Wu 2 participants 5 messages Sep 10, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 53/62 mm/memcontrol.c:755:6: sparse: symbol 'memcg_flush_cache_create_qu

Hi Glauber,

FYI, there are new sparse warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/glommer/memcg.git kmemcg-slab
head:   b20a4abf14992a969b761691f8978a28cacdecb4
commit: db85c41ae676eb65f37b31a59111f2b9b56621ed [53/62] memcg: infrastructure to match an allocation to the right cache

All sparse warnings:

mm/memcontrol.c:585:12: sparse: symbol 'cache_types' was not declared. Should it be static?
+ mm/memcontrol.c:755:6: sparse: symbol 'memcg_flush_cache_create_queue' was not declared. Should it be static? include/linux/cgroup.h:559:16: sparse: cast removes address space of expression include/linux/cgroup.h:559:16: sparse: incompatible types in comparison expression (different address spaces) include/linux/cgroup.h:559:16: sparse: incompatible types in comparison expression (different address spaces) mm/memcontrol.c:4763:21: sparse: incompatible types in comparison expression (different address spaces) mm/memcontrol.c:4765:21: sparse: incompatible types in comparison expression (different address spaces) mm/memcontrol.c:5946:31: sparse: incompatible types in comparison expression (different address spaces)

vim +755 mm/memcontrol.c

748 /*
749 * Flush the queue of kmem_caches to create, because we're creating a cgroup.
750 *
751 * We might end up flushing other cgroups' creation requests as well, but
752 * they will just get queued again next time someone tries to make a slab
753 * allocation for them.
754 */

Quoted 1 line

755 void memcg_flush_cache_create_queue(void)

756 {
757 struct create_work *cw, *tmp;
758 unsigned long flags;
759
760 spin_lock_irqsave(&cache_queue_lock, flags);
761 list_for_each_entry_safe(cw, tmp, &create_queue, list) {
762 list_del(&cw->list);
763 kfree(cw);
764 }
765 spin_unlock_irqrestore(&cache_queue_lock, flags);
766 }

Please consider folding the attached diff and ignore conflicts with the previous one :-)

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu <[email protected]> Intel Corporation

mm/memcontrol.c +2 -2
index b85814a..fb7f1ca 100644
@@ -582,7 +582,7 @@ static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,
 	return new;
 }
 
-struct ida cache_types;
+static struct ida cache_types;
 
 void memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *cachep)
 {
@@ -752,7 +752,7 @@ static LIST_HEAD(create_queue);
  * they will just get queued again next time someone tries to make a slab
  * allocation for them.
  */
-void memcg_flush_cache_create_queue(void)
+static void memcg_flush_cache_create_queue(void)
 {
 	struct create_work *cw, *tmp;
 	unsigned long flags;
#1 lore raw

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

Glauber,

The patch entitled

sl[au]b: Allocate objects from memcg cache

changes

 include/linux/slub_def.h |   15 ++++++++++-----

which triggers this warning:

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

It's the MAX_LEVEL that is defined in include/linux/idr.h.

MAX_LEVEL is obviously too generic. Better adding some prefix to it?

Thanks,
Fengguang

#2 lore raw

Re: [glommer-memcg:kmemcg-slab 57/62] drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Quoted 10 lines · Fengguang Wu

Glauber,

The patch entitled

sl[au]b: Allocate objects from memcg cache

changes

 include/linux/slub_def.h |   15 ++++++++++-----
Quoted 8 lines

which triggers this warning:

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

It's the MAX_LEVEL that is defined in include/linux/idr.h.

MAX_LEVEL is obviously too generic. Better adding some prefix to it?

I don't see any MAX_LEVEL definition in this patch. You say it is defined in include/linux/idr.h, and as the diffstat shows, I am not touching this file.

I think this needs patching independently.

#3 lore raw

Re: [glommer-memcg:kmemcg-slab 57/62] drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Quoted 15 lines · Glauber Costa
Quoted 14 lines · Fengguang Wu

Glauber,

The patch entitled

sl[au]b: Allocate objects from memcg cache

changes

 include/linux/slub_def.h |   15 ++++++++++-----

which triggers this warning:

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

Quoted 8 lines
Quoted 4 lines

It's the MAX_LEVEL that is defined in include/linux/idr.h.

MAX_LEVEL is obviously too generic. Better adding some prefix to it?

I don't see any MAX_LEVEL definition in this patch. You say it is defined in include/linux/idr.h, and as the diffstat shows, I am not touching this file.

It's a rather *unexpected* side effect. You changed slub_def.h to include memcontrol.h/cgroup.h which in turn includes idr.h.

Quoted 1 line

I think this needs patching independently.

Yes, sure. And perhaps send it for quick inclusion before your patches?

Thanks,
Fengguang

#4 lore raw

Re: [glommer-memcg:kmemcg-slab 57/62] drivers/video/riva/fbdev.c:281:9: sparse: preprocessor token MAX_LEVEL redefined

Quoted 21 lines · Fengguang Wu
Quoted 20 lines · Glauber Costa
Quoted 18 lines · Fengguang Wu

Glauber,

The patch entitled

sl[au]b: Allocate objects from memcg cache

changes

 include/linux/slub_def.h |   15 ++++++++++-----

which triggers this warning:

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

It's the MAX_LEVEL that is defined in include/linux/idr.h.

MAX_LEVEL is obviously too generic. Better adding some prefix to it?

Quoted 7 lines
Quoted 3 lines

I don't see any MAX_LEVEL definition in this patch. You say it is defined in include/linux/idr.h, and as the diffstat shows, I am not touching this file.

It's a rather *unexpected* side effect. You changed slub_def.h to include memcontrol.h/cgroup.h which in turn includes idr.h.

Indeed.

Quoted 4 lines
Quoted 1 line

I think this needs patching independently.

Yes, sure. And perhaps send it for quick inclusion before your patches?

I agree.