Skip to contentSkip to conversation

cgroup debug seems dead?

DISCUSSION Stephen Hemminger 4 participants 10 messages Oct 19, 2010 – Oct 20, 2010 4 messages not in the archive

lore
Share & tools
lore
#0 lore raw

While running namespace checks to look for dead code, I found that debug_subsys is declared global but never used in current code.

This leads to the fact that since debug_subsys is never used, the whole set of debugging functions is also never used. Is the whole CGROUP_DEBUG config option dead?

#1 lore raw
Quoted 8 lines · Stephen Hemminger

While running namespace checks to look for dead code, I found that debug_subsys is declared global but never used in current code.

This leads to the fact that since debug_subsys is never used, the whole set of debugging functions is also never used. Is the whole CGROUP_DEBUG config option dead?

No, it's not.

The debug code has been moved from kernel/cgroup_debug.c to kernel/cgroup.c, but the config and the code is not dead.

Is it a false positive of that check? The pointer to debug_subsys is stored in an array, and the array will be iterated in some places, so debug_subsys will be accessed.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 10 lines · On Wed, 20 Oct 2010 09:11:10 +0800 Li Zefan…
Quoted 8 lines · Stephen Hemminger

While running namespace checks to look for dead code, I found that debug_subsys is declared global but never used in current code.

This leads to the fact that since debug_subsys is never used, the whole set of debugging functions is also never used. Is the whole CGROUP_DEBUG config option dead?

Quoted 8 lines

No, it's not.

The debug code has been moved from kernel/cgroup_debug.c to kernel/cgroup.c, but the config and the code is not dead.

Is it a false positive of that check? The pointer to debug_subsys is stored in an array, and the array will be iterated in some places, so debug_subsys will be accessed.

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys
kernel/cgroup.c: kfree(cont->subsys[debug_subsys_id]);
kernel/cgroup.c:struct cgroup_subsys debug_subsys = {
kernel/cgroup.c: .subsys_id = debug_subsys_id,

There does not appear to be any place that structure is referenced.

#4 lore raw
Quoted 7 lines · Stephen Hemminger

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys kernel/cgroup.c:        kfree(cont->subsys[debug_subsys_id]); kernel/cgroup.c:struct cgroup_subsys debug_subsys = { kernel/cgroup.c:        .subsys_id = debug_subsys_id,

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

A message that is not in the archive (sent off-list or not yet received): AANLkTinH2t41jvGG_D9qAdN5MvRHg7aR+J2mKV5ifMfC-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#6 lore raw
Quoted 5 lines · On Tue, 19 Oct 2010 18:14:10 -0700 Paul Menage…
Quoted 3 lines · Stephen Hemminger

Where is it stored in an array?

Quoted 8 lines
Quoted 4 lines

~/kernel/linux-2.6$ git grep debug_subsys kernel/cgroup.c:        kfree(cont->subsys[debug_subsys_id]); kernel/cgroup.c:struct cgroup_subsys debug_subsys = { kernel/cgroup.c:        .subsys_id = debug_subsys_id,

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

That would work but doesn't because the following is missing in cgroup_subsys.h!

#ifdef CONFIG_CGROUP_DEBUG
SUBSYS(debug)
#endif

/* */

#8 lore raw
In reply to Stephen Hemminger
Quoted 11 lines · Stephen Hemminger
Quoted 8 lines · On Tue, 19 Oct 2010 18:14:10 -0700 Paul Menage…
Quoted 6 lines · Stephen Hemminger

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys
kernel/cgroup.c: kfree(cont->subsys[debug_subsys_id]);
kernel/cgroup.c:struct cgroup_subsys debug_subsys = {
kernel/cgroup.c: .subsys_id = debug_subsys_id,

Quoted 6 lines
Quoted 3 lines

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

That would work but doesn't because the following is missing in cgroup_subsys.h!

It's there in cgroup_subsys.h (2.6.36-rc8). What kernel are you using?

#ifdef CONFIG_CPUSETS
SUBSYS(cpuset)
#endif

/* */

#ifdef CONFIG_CGROUP_DEBUG
SUBSYS(debug)
#endif

/* */

#ifdef CONFIG_CGROUP_NS
SUBSYS(ns)
#endif

Quoted 5 lines

#ifdef CONFIG_CGROUP_DEBUG
SUBSYS(debug)
#endif

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw
Quoted 12 lines · On Wed, 20 Oct 2010 09:43:06 +0800 Li Zefan…
Quoted 11 lines · Stephen Hemminger
Quoted 8 lines · On Tue, 19 Oct 2010 18:14:10 -0700 Paul Menage…
Quoted 6 lines · Stephen Hemminger

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys
kernel/cgroup.c: kfree(cont->subsys[debug_subsys_id]);
kernel/cgroup.c:struct cgroup_subsys debug_subsys = {
kernel/cgroup.c: .subsys_id = debug_subsys_id,

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

That would work but doesn't because the following is missing in cgroup_subsys.h!

It's there in cgroup_subsys.h (2.6.36-rc8). What kernel are you using?

Found it, thanks. Still not sure why the tools were confused.

#11 lore raw
Quoted 15 lines · Stephen Hemminger
Quoted 12 lines · On Wed, 20 Oct 2010 09:43:06 +0800 Li Zefan…
Quoted 11 lines · Stephen Hemminger
Quoted 8 lines · On Tue, 19 Oct 2010 18:14:10 -0700 Paul Menage…
Quoted 6 lines · Stephen Hemminger

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys
kernel/cgroup.c: kfree(cont->subsys[debug_subsys_id]);
kernel/cgroup.c:struct cgroup_subsys debug_subsys = {
kernel/cgroup.c: .subsys_id = debug_subsys_id,

Quoted 9 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

That would work but doesn't because the following is missing in cgroup_subsys.h!

It's there in cgroup_subsys.h (2.6.36-rc8). What kernel are you using?

Found it, thanks. Still not sure why the tools were confused.

That's normal. Think about how many false positives we can get with checkpatch.pl.

Your tools should be static code analyzers, and the macro we use in cgroup is too complex for them.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#13 lore raw
Quoted 23 lines · Li Zefan
Quoted 22 lines · Stephen Hemminger
Quoted 18 lines · On Wed, 20 Oct 2010 09:43:06 +0800 Li Zefan…
Quoted 11 lines · Stephen Hemminger
Quoted 8 lines · On Tue, 19 Oct 2010 18:14:10 -0700 Paul Menage…
Quoted 6 lines · Stephen Hemminger

Where is it stored in an array?

~/kernel/linux-2.6$ git grep debug_subsys
kernel/cgroup.c: kfree(cont->subsys[debug_subsys_id]);
kernel/cgroup.c:struct cgroup_subsys debug_subsys = {
kernel/cgroup.c: .subsys_id = debug_subsys_id,

Quoted 5 lines
Quoted 3 lines

It's via the include of cgroup_subsys.h at the start of kernel/cgroup.c

Paul

That would work but doesn't because the following is missing in cgroup_subsys.h!

It's there in cgroup_subsys.h (2.6.36-rc8). What kernel are you using?

Quoted 8 lines
Quoted 2 lines

Found it, thanks. Still not sure why the tools were confused.

That's normal. Think about how many false positives we can get with checkpatch.pl.

Your tools should be static code analyzers, and the macro we use in cgroup is too complex for them.

namespace.pl is not that static. Becuase it checks the binary not the source, a little more dynamic that you think. ;)

Stephen, what message did you see?