All of lore.kernel.org
 help / color / mirror / Atom feed
* cgroup debug seems dead?
@ 2010-10-19 15:51 Stephen Hemminger
  2010-10-20  1:11 ` Li Zefan
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2010-10-19 15:51 UTC (permalink / raw)
  To: Paul Menage, Li Zefan
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
  2010-10-19 15:51 cgroup debug seems dead? Stephen Hemminger
@ 2010-10-20  1:11 ` Li Zefan
       [not found]   ` <4CBE41AE.2020101-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Li Zefan @ 2010-10-20  1:11 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paul Menage,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Stephen Hemminger wrote:
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
       [not found]   ` <4CBE41AE.2020101-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2010-10-20  1:12     ` Stephen Hemminger
  2010-10-20  1:14       ` Paul Menage
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2010-10-20  1:12 UTC (permalink / raw)
  To: Li Zefan
  Cc: Paul Menage,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Wed, 20 Oct 2010 09:11:10 +0800
Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:

> Stephen Hemminger wrote:
> > 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.

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.
-- 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
  2010-10-20  1:12     ` Stephen Hemminger
@ 2010-10-20  1:14       ` Paul Menage
       [not found]         ` <AANLkTinH2t41jvGG_D9qAdN5MvRHg7aR+J2mKV5ifMfC-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Menage @ 2010-10-20  1:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
<shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
>
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
       [not found]         ` <AANLkTinH2t41jvGG_D9qAdN5MvRHg7aR+J2mKV5ifMfC-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-20  1:37           ` Stephen Hemminger
  2010-10-20  1:42             ` Paul Menage
  2010-10-20  1:43             ` Li Zefan
  0 siblings, 2 replies; 10+ messages in thread
From: Stephen Hemminger @ 2010-10-20  1:37 UTC (permalink / raw)
  To: Paul Menage; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Tue, 19 Oct 2010 18:14:10 -0700
Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:

> On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
> <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
> >
> > 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

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

#ifdef CONFIG_CGROUP_DEBUG
SUBSYS(debug)
#endif

/* */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
  2010-10-20  1:37           ` Stephen Hemminger
@ 2010-10-20  1:42             ` Paul Menage
  2010-10-20  1:43             ` Li Zefan
  1 sibling, 0 replies; 10+ messages in thread
From: Paul Menage @ 2010-10-20  1:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Tue, Oct 19, 2010 at 6:37 PM, Stephen Hemminger
<shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
>
> That would work but doesn't because the following is missing
> in cgroup_subsys.h!
>
> #ifdef CONFIG_CGROUP_DEBUG
> SUBSYS(debug)
> #endif
>
> /* */
>

I see that fragment at line 16 in cgroup_subsys.h (see e.g.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=include/linux/cgroup_subsys.h;h=ccefff02b6cb005123c4b904e06116cb3a850474;hb=HEAD)

Which kernel tree are you looking at?

Paul

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
  2010-10-20  1:37           ` Stephen Hemminger
  2010-10-20  1:42             ` Paul Menage
@ 2010-10-20  1:43             ` Li Zefan
       [not found]               ` <4CBE492A.3020208-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Li Zefan @ 2010-10-20  1:43 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paul Menage,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Stephen Hemminger wrote:
> On Tue, 19 Oct 2010 18:14:10 -0700
> Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
>> <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
>>> 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
> 
> 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

> 
> #ifdef CONFIG_CGROUP_DEBUG
> SUBSYS(debug)
> #endif
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
       [not found]               ` <4CBE492A.3020208-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2010-10-20  2:14                 ` Stephen Hemminger
  2010-10-20  2:23                   ` Li Zefan
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2010-10-20  2:14 UTC (permalink / raw)
  To: Li Zefan
  Cc: Paul Menage,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Wed, 20 Oct 2010 09:43:06 +0800
Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:

> Stephen Hemminger wrote:
> > On Tue, 19 Oct 2010 18:14:10 -0700
> > Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> > 
> >> On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
> >> <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
> >>> 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
> > 
> > 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
  2010-10-20  2:14                 ` Stephen Hemminger
@ 2010-10-20  2:23                   ` Li Zefan
       [not found]                     ` <4CBE52AE.2010409-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Li Zefan @ 2010-10-20  2:23 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paul Menage,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Stephen Hemminger wrote:
> On Wed, 20 Oct 2010 09:43:06 +0800
> Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:
> 
>> Stephen Hemminger wrote:
>>> On Tue, 19 Oct 2010 18:14:10 -0700
>>> Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>
>>>> On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
>>>> <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
>>>>> 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
>>> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: cgroup debug seems dead?
       [not found]                     ` <4CBE52AE.2010409-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2010-10-20  4:49                       ` Américo Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Américo Wang @ 2010-10-20  4:49 UTC (permalink / raw)
  To: Li Zefan
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Stephen Hemminger, Paul Menage

On Wed, Oct 20, 2010 at 10:23:42AM +0800, Li Zefan wrote:
>Stephen Hemminger wrote:
>> On Wed, 20 Oct 2010 09:43:06 +0800
>> Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:
>> 
>>> Stephen Hemminger wrote:
>>>> On Tue, 19 Oct 2010 18:14:10 -0700
>>>> Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>>
>>>>> On Tue, Oct 19, 2010 at 6:12 PM, Stephen Hemminger
>>>>> <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org> wrote:
>>>>>> 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
>>>> 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.

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-10-20  4:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-19 15:51 cgroup debug seems dead? Stephen Hemminger
2010-10-20  1:11 ` Li Zefan
     [not found]   ` <4CBE41AE.2020101-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2010-10-20  1:12     ` Stephen Hemminger
2010-10-20  1:14       ` Paul Menage
     [not found]         ` <AANLkTinH2t41jvGG_D9qAdN5MvRHg7aR+J2mKV5ifMfC-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-20  1:37           ` Stephen Hemminger
2010-10-20  1:42             ` Paul Menage
2010-10-20  1:43             ` Li Zefan
     [not found]               ` <4CBE492A.3020208-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2010-10-20  2:14                 ` Stephen Hemminger
2010-10-20  2:23                   ` Li Zefan
     [not found]                     ` <4CBE52AE.2010409-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2010-10-20  4:49                       ` Américo Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.