2009-09-12 06:53:06

by Stephen Rothwell

[permalink] [raw]
Subject: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

Hi Steve,

On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <[email protected]> wrote:
>
> On Tue, 2009-08-04 at 16:16 +1000, Stephen Rothwell wrote:
> >
> > Today's linux-next build (powerpc ppc64_defconfig) produced these warnings:
> >
> > kernel/trace/ring_buffer.c: In function 'rb_head_page_set':
> > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > kernel/trace/ring_buffer.c: In function 'rb_head_page_replace':
> > kernel/trace/ring_buffer.c:797: warning: initialization makes integer from pointer without a cast
> >
> > Introduced by commit 77ae365eca895061c8bf2b2e3ae1d9ea62869739
> > ("ring-buffer: make lockless").
>
> Thanks, I'll take a look at it.

Now that this is in Linus' tree, can we have a fix for the waning, please?
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (0.98 kB)
(No filename) (198.00 B)
Download all attachments

2009-09-12 07:39:32

by Ingo Molnar

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)


* Stephen Rothwell <[email protected]> wrote:

> Hi Steve,
>
> On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <[email protected]> wrote:
> >
> > On Tue, 2009-08-04 at 16:16 +1000, Stephen Rothwell wrote:
> > >
> > > Today's linux-next build (powerpc ppc64_defconfig) produced these warnings:
> > >
> > > kernel/trace/ring_buffer.c: In function 'rb_head_page_set':
> > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > kernel/trace/ring_buffer.c: In function 'rb_head_page_replace':
> > > kernel/trace/ring_buffer.c:797: warning: initialization makes integer from pointer without a cast
> > >
> > > Introduced by commit 77ae365eca895061c8bf2b2e3ae1d9ea62869739
> > > ("ring-buffer: make lockless").
> >
> > Thanks, I'll take a look at it.
>
> Now that this is in Linus' tree, can we have a fix for the waning,
> please?

The first warning got fixed 1.5 months ago - the second one at line
797 is still there but harmless - you can ignore it for now, it will
be fixed.

Ingo

2009-09-12 10:46:37

by Stephen Rothwell

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

Hi Ingo,

On Sat, 12 Sep 2009 09:39:06 +0200 Ingo Molnar <[email protected]> wrote:
>
> * Stephen Rothwell <[email protected]> wrote:
>
> > On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <[email protected]> wrote:
> > >
> > > On Tue, 2009-08-04 at 16:16 +1000, Stephen Rothwell wrote:
> > > >
> > > > Today's linux-next build (powerpc ppc64_defconfig) produced these warnings:
> > > >
> > > > kernel/trace/ring_buffer.c: In function 'rb_head_page_set':
> > > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > > kernel/trace/ring_buffer.c: In function 'rb_head_page_replace':
> > > > kernel/trace/ring_buffer.c:797: warning: initialization makes integer from pointer without a cast
> > > >
> > > > Introduced by commit 77ae365eca895061c8bf2b2e3ae1d9ea62869739
> > > > ("ring-buffer: make lockless").
> > >
> > > Thanks, I'll take a look at it.
> >
> > Now that this is in Linus' tree, can we have a fix for the waning,
> > please?
>
> The first warning got fixed 1.5 months ago - the second one at line
> 797 is still there but harmless - you can ignore it for now, it will
> be fixed.

Well linux-next builds still gave both warnings yesterday, so I was just
asking to make sure they were not forgotten.

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (1.45 kB)
(No filename) (198.00 B)
Download all attachments

2009-09-12 11:13:14

by Jaswinder Singh Rajput

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> * Stephen Rothwell <[email protected]> wrote:
>
> > Hi Steve,
> >
> > On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <[email protected]> wrote:
> > >
> > > On Tue, 2009-08-04 at 16:16 +1000, Stephen Rothwell wrote:
> > > >
> > > > Today's linux-next build (powerpc ppc64_defconfig) produced these warnings:
> > > >
> > > > kernel/trace/ring_buffer.c: In function 'rb_head_page_set':
> > > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > > kernel/trace/ring_buffer.c:704: warning: initialization makes pointer from integer without a cast
> > > > kernel/trace/ring_buffer.c: In function 'rb_head_page_replace':
> > > > kernel/trace/ring_buffer.c:797: warning: initialization makes integer from pointer without a cast
> > > >
> > > > Introduced by commit 77ae365eca895061c8bf2b2e3ae1d9ea62869739
> > > > ("ring-buffer: make lockless").
> > >
> > > Thanks, I'll take a look at it.
> >
> > Now that this is in Linus' tree, can we have a fix for the waning,
> > please?
>
> The first warning got fixed 1.5 months ago - the second one at line
> 797 is still there but harmless - you can ignore it for now, it will
> be fixed.
>

Here are some more trace related warnings in current linus (as well as
-tip) tree :

CHECK arch/x86/kernel/ptrace.c
include/trace/events/syscalls.h:18:1: warning: symbol 'ftrace_raw_output_sys_enter' was not declared. Should it be static?
include/trace/events/syscalls.h:42:1: warning: symbol 'ftrace_raw_output_sys_exit' was not declared. Should it be static?
include/trace/events/syscalls.h:18:1: warning: symbol 'ftrace_define_fields_sys_enter' was not declared. Should it be static?
include/trace/events/syscalls.h:42:1: warning: symbol 'ftrace_define_fields_sys_exit' was not declared. Should it be static?
include/trace/events/syscalls.h:18:1: error: bad constant expression
include/trace/events/syscalls.h:42:1: error: bad constant expression

CHECK kernel/sched.c
include/trace/events/sched.h:152:1: warning: symbol 'flags' shadows an earlier one
include/trace/events/sched.h:152:1: originally declared here
include/trace/events/sched.h:13:1: warning: symbol 'ftrace_raw_output_sched_kthread_stop' was not declared. Should it be static?
include/trace/events/sched.h:35:1: warning: symbol 'ftrace_raw_output_sched_kthread_stop_ret' was not declared. Should it be static?
include/trace/events/sched.h:58:1: warning: symbol 'ftrace_raw_output_sched_wait_task' was not declared. Should it be static?
include/trace/events/sched.h:86:1: warning: symbol 'ftrace_raw_output_sched_wakeup' was not declared. Should it be static?
include/trace/events/sched.h:119:1: warning: symbol 'ftrace_raw_output_sched_wakeup_new' was not declared. Should it be static?
include/trace/events/sched.h:152:1: warning: symbol 'ftrace_raw_output_sched_switch' was not declared. Should it be static?
include/trace/events/sched.h:192:1: warning: symbol 'ftrace_raw_output_sched_migrate_task' was not declared. Should it be static?
include/trace/events/sched.h:222:1: warning: symbol 'ftrace_raw_output_sched_process_free' was not declared. Should it be static?
include/trace/events/sched.h:247:1: warning: symbol 'ftrace_raw_output_sched_process_exit' was not declared. Should it be static?
include/trace/events/sched.h:272:1: warning: symbol 'ftrace_raw_output_sched_process_wait' was not declared. Should it be static?
include/trace/events/sched.h:297:1: warning: symbol 'ftrace_raw_output_sched_process_fork' was not declared. Should it be static?
include/trace/events/sched.h:325:1: warning: symbol 'ftrace_raw_output_sched_signal_send' was not declared. Should it be static?
include/trace/events/sched.h:356:1: warning: symbol 'ftrace_raw_output_sched_stat_wait' was not declared. Should it be static?
include/trace/events/sched.h:386:1: warning: symbol 'ftrace_raw_output_sched_stat_sleep' was not declared. Should it be static?
include/trace/events/sched.h:416:1: warning: symbol 'ftrace_raw_output_sched_stat_iowait' was not declared. Should it be static?
include/trace/events/sched.h:13:1: warning: symbol 'ftrace_define_fields_sched_kthread_stop' was not declared. Should it be static?
include/trace/events/sched.h:35:1: warning: symbol 'ftrace_define_fields_sched_kthread_stop_ret' was not declared. Should it be static?
include/trace/events/sched.h:58:1: warning: symbol 'ftrace_define_fields_sched_wait_task' was not declared. Should it be static?
include/trace/events/sched.h:86:1: warning: symbol 'ftrace_define_fields_sched_wakeup' was not declared. Should it be static?
include/trace/events/sched.h:119:1: warning: symbol 'ftrace_define_fields_sched_wakeup_new' was not declared. Should it be static?
include/trace/events/sched.h:152:1: warning: symbol 'ftrace_define_fields_sched_switch' was not declared. Should it be static?
include/trace/events/sched.h:192:1: warning: symbol 'ftrace_define_fields_sched_migrate_task' was not declared. Should it be static?
include/trace/events/sched.h:222:1: warning: symbol 'ftrace_define_fields_sched_process_free' was not declared. Should it be static?
include/trace/events/sched.h:247:1: warning: symbol 'ftrace_define_fields_sched_process_exit' was not declared. Should it be static?
include/trace/events/sched.h:272:1: warning: symbol 'ftrace_define_fields_sched_process_wait' was not declared. Should it be static?
include/trace/events/sched.h:297:1: warning: symbol 'ftrace_define_fields_sched_process_fork' was not declared. Should it be static?
include/trace/events/sched.h:325:1: warning: symbol 'ftrace_define_fields_sched_signal_send' was not declared. Should it be static?
include/trace/events/sched.h:356:1: warning: symbol 'ftrace_define_fields_sched_stat_wait' was not declared. Should it be static?
include/trace/events/sched.h:386:1: warning: symbol 'ftrace_define_fields_sched_stat_sleep' was not declared. Should it be static?
include/trace/events/sched.h:416:1: warning: symbol 'ftrace_define_fields_sched_stat_iowait' was not declared. Should it be static?
include/trace/events/sched.h:13:1: error: bad constant expression
include/trace/events/sched.h:35:1: error: bad constant expression
include/trace/events/sched.h:58:1: error: bad constant expression
include/trace/events/sched.h:86:1: error: bad constant expression
include/trace/events/sched.h:119:1: error: bad constant expression
include/trace/events/sched.h:152:1: error: bad constant expression
include/trace/events/sched.h:192:1: error: bad constant expression
include/trace/events/sched.h:222:1: error: bad constant expression
include/trace/events/sched.h:247:1: error: bad constant expression
include/trace/events/sched.h:272:1: error: bad constant expression
include/trace/events/sched.h:297:1: error: bad constant expression
include/trace/events/sched.h:325:1: error: bad constant expression
include/trace/events/sched.h:356:1: error: bad constant expression
include/trace/events/sched.h:386:1: error: bad constant expression
include/trace/events/sched.h:416:1: error: bad constant expression

CHECK kernel/softirq.c
include/trace/events/irq.h:34:1: warning: symbol 'ftrace_raw_output_irq_handler_entry' was not declared. Should it be static?
include/trace/events/irq.h:64:1: warning: symbol 'ftrace_raw_output_irq_handler_exit' was not declared. Should it be static?
include/trace/events/irq.h:95:1: warning: symbol 'ftrace_raw_output_softirq_entry' was not declared. Should it be static?
include/trace/events/irq.h:124:1: warning: symbol 'ftrace_raw_output_softirq_exit' was not declared. Should it be static?
include/trace/events/irq.h:34:1: warning: symbol 'ftrace_define_fields_irq_handler_entry' was not declared. Should it be static?
include/trace/events/irq.h:64:1: warning: symbol 'ftrace_define_fields_irq_handler_exit' was not declared. Should it be static?
include/trace/events/irq.h:95:1: warning: symbol 'ftrace_define_fields_softirq_entry' was not declared. Should it be static?
include/trace/events/irq.h:124:1: warning: symbol 'ftrace_define_fields_softirq_exit' was not declared. Should it be static?
include/trace/events/irq.h:34:1: error: bad constant expression
include/trace/events/irq.h:64:1: error: bad constant expression
include/trace/events/irq.h:95:1: error: bad constant expression
include/trace/events/irq.h:124:1: error: bad constant expression

CHECK kernel/ptrace.c
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:532:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:538:32: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:543:33: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
kernel/ptrace.c:653:9: warning: cast adds address space to expression (<asn:1>)
include/linux/sched.h:2192:2: warning: context problem in 'ptrace_getsiginfo': '_spin_unlock_irqrestore' expected different context
include/linux/sched.h:2192:2: context 'lock': wanted >= 1, got 0
include/linux/sched.h:2192:2: warning: context problem in 'ptrace_setsiginfo': '_spin_unlock_irqrestore' expected different context
include/linux/sched.h:2192:2: context 'lock': wanted >= 1, got 0

CHECK kernel/workqueue.c
include/trace/events/workqueue.h:11:1: warning: symbol 'ftrace_raw_output_workqueue_insertion' was not declared. Should it be static?
include/trace/events/workqueue.h:33:1: warning: symbol 'ftrace_raw_output_workqueue_execution' was not declared. Should it be static?
include/trace/events/workqueue.h:56:1: warning: symbol 'ftrace_raw_output_workqueue_creation' was not declared. Should it be static?
include/trace/events/workqueue.h:78:1: warning: symbol 'ftrace_raw_output_workqueue_destruction' was not declared. Should it be static?
include/trace/events/workqueue.h:11:1: error: incompatible types for operation (<)
include/trace/events/workqueue.h:11:1: left side has type void ( *[usertype] <noident> )( ... )
include/trace/events/workqueue.h:11:1: right side has type int
include/trace/events/workqueue.h:33:1: error: incompatible types for operation (<)
include/trace/events/workqueue.h:33:1: left side has type void ( *[usertype] <noident> )( ... )
include/trace/events/workqueue.h:33:1: right side has type int
include/trace/events/workqueue.h:11:1: error: bad constant expression
include/trace/events/workqueue.h:33:1: error: bad constant expression
include/trace/events/workqueue.h:56:1: error: bad constant expression
include/trace/events/workqueue.h:78:1: error: bad constant expression

CHECK kernel/trace/ring_buffer.c
kernel/trace/ring_buffer.c:1752:2: warning: do-while statement is not a compound statement
kernel/trace/ring_buffer.c:1917:2: warning: do-while statement is not a compound statement
kernel/trace/ring_buffer.c:1752:2: error: bad constant expression
kernel/trace/ring_buffer.c:1752:2: error: cannot size expression
kernel/trace/ring_buffer.c:1917:2: error: bad constant expression
kernel/trace/ring_buffer.c:1917:2: error: cannot size expression

CHECK kernel/trace/trace.c
kernel/trace/trace.c:3721:16: warning: symbol 'd_tracer' shadows an earlier one
kernel/trace/trace.c:3693:22: originally declared here
kernel/trace/trace.c:3744:16: warning: symbol 'd_percpu' shadows an earlier one
kernel/trace/trace.c:3716:22: originally declared here
kernel/trace/trace.c:3937:16: warning: symbol 'd_tracer' shadows an earlier one
kernel/trace/trace.c:3693:22: originally declared here
kernel/trace/trace.c:4051:16: warning: symbol 'd_tracer' shadows an earlier one
kernel/trace/trace.c:3693:22: originally declared here
kernel/trace/trace.c:2492:30: error: bad constant expression
kernel/trace/trace.c:2642:30: error: bad constant expression

CHECK kernel/trace/trace_events.c
kernel/trace/trace_events.c:1190:23: warning: symbol 'trace_module_nb' was not declared. Should it be static?

CHECK kernel/trace/trace_export.c
kernel/trace/trace_event_types.h:63:1: warning: symbol 'event_special' was not declared. Should it be static?
kernel/trace/trace_event_types.h:108:1: error: incompatible types for operation (<)
kernel/trace/trace_event_types.h:108:1: left side has type char *<noident>
kernel/trace/trace_event_types.h:108:1: right side has type int
kernel/trace/trace_event_types.h:155:1: error: incompatible types for operation (<)
kernel/trace/trace_event_types.h:155:1: left side has type void const *<noident>
kernel/trace/trace_event_types.h:155:1: right side has type int
kernel/trace/trace_event_types.h:169:1: error: incompatible types for operation (<)
kernel/trace/trace_event_types.h:169:1: left side has type void const *<noident>
kernel/trace/trace_event_types.h:169:1: right side has type int

CHECK kernel/trace/trace_event_profile.c
kernel/trace/trace_event_profile.c:10:5: warning: symbol 'ftrace_profile_enable' was not declared. Should it be static?
kernel/trace/trace_event_profile.c:27:6: warning: symbol 'ftrace_profile_disable' was not declared. Should it be static?

CHECK kernel/trace/trace_events_filter.c
kernel/trace/trace_events_filter.c:634:44: warning: incorrect type in argument 3 (different signedness)
kernel/trace/trace_events_filter.c:634:44: expected long long *<noident>
kernel/trace/trace_events_filter.c:634:44: got unsigned long long *<noident>

CHECK kernel/lockdep.c
include/trace/events/lockdep.h:12:1: warning: symbol 'ftrace_raw_output_lock_acquire' was not declared. Should it be static?
include/trace/events/lockdep.h:35:1: warning: symbol 'ftrace_raw_output_lock_release' was not declared. Should it be static?
include/trace/events/lockdep.h:54:1: warning: symbol 'ftrace_raw_output_lock_contended' was not declared. Should it be static?
include/trace/events/lockdep.h:71:1: warning: symbol 'ftrace_raw_output_lock_acquired' was not declared. Should it be static?
include/trace/events/lockdep.h:12:1: warning: symbol 'ftrace_define_fields_lock_acquire' was not declared. Should it be static?
include/trace/events/lockdep.h:35:1: warning: symbol 'ftrace_define_fields_lock_release' was not declared. Should it be static?
include/trace/events/lockdep.h:54:1: warning: symbol 'ftrace_define_fields_lock_contended' was not declared. Should it be static?
include/trace/events/lockdep.h:71:1: warning: symbol 'ftrace_define_fields_lock_acquired' was not declared. Should it be static?
include/trace/events/lockdep.h:12:1: error: bad constant expression
include/trace/events/lockdep.h:35:1: error: bad constant expression
include/trace/events/lockdep.h:54:1: error: bad constant expression
include/trace/events/lockdep.h:71:1: error: bad constant expression

CHECK kernel/module.c
include/trace/events/module.h:18:1: warning: symbol 'flags' shadows an earlier one
include/trace/events/module.h:18:1: originally declared here
kernel/module.c:247:20: warning: symbol 'arr' shadows an earlier one
kernel/module.c:224:25: originally declared here
kernel/module.c:2281:14: warning: symbol 'strtab' shadows an earlier one
kernel/module.c:1948:38: originally declared here
include/trace/events/module.h:98:1: error: cannot size expression
include/trace/events/module.h:98:1: error: cannot size expression
include/trace/events/module.h:18:1: error: bad constant expression
include/trace/events/module.h:37:1: error: bad constant expression
include/trace/events/module.h:54:1: error: bad constant expression
include/trace/events/module.h:76:1: error: bad constant expression
include/trace/events/module.h:98:1: error: bad constant expression

CHECK kernel/tracepoint.c
kernel/tracepoint.c:559:5: warning: symbol 'tracepoint_module_notify' was not declared. Should it be static?
kernel/tracepoint.c:574:23: warning: symbol 'tracepoint_module_nb' was not declared. Should it be static?
kernel/tracepoint.c:592:6: warning: symbol 'syscall_regfunc' was not declared. Should it be static?
kernel/tracepoint.c:609:6: warning: symbol 'syscall_unregfunc' was not declared. Should it be static?

And some includecheck warnings in -tip :

arch/x86/kernel/dumpstack.c: linux/ftrace.h is included more than once.
arch/x86/kernel/syscall_64.c: asm/unistd_64.h is included more than once.
arch/x86/kernel/traps.c: asm/traps.h is included more than once.
arch/x86/kvm/mmu.c: paging_tmpl.h is included more than once.
arch/x86/mm/kmemcheck/shadow.c: linux/module.h is included more than once.
arch/x86/vdso/vma.c: vextern.h is included more than once.

I have send various patches to fix some of these warnings but no action taken.

Thanks,
--
JSR

2009-09-14 13:50:37

by Steven Rostedt

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> * Stephen Rothwell <[email protected]> wrote:

> > Now that this is in Linus' tree, can we have a fix for the waning,
> > please?
>
> The first warning got fixed 1.5 months ago - the second one at line
> 797 is still there but harmless - you can ignore it for now, it will
> be fixed.

The first warning never got fixed. The typecast was placed in the wrong
spot (my fault). I sent another patch that fixes both warnings.

-- Steve

2009-09-14 13:53:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)


* Steven Rostedt <[email protected]> wrote:

> On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> > * Stephen Rothwell <[email protected]> wrote:
>
> > > Now that this is in Linus' tree, can we have a fix for the waning,
> > > please?
> >
> > The first warning got fixed 1.5 months ago - the second one at line
> > 797 is still there but harmless - you can ignore it for now, it will
> > be fixed.
>
> The first warning never got fixed. The typecast was placed in the
> wrong spot (my fault). I sent another patch that fixes both warnings.

Pulled it, thanks Steve.

Ingo

2009-09-14 15:20:25

by Steven Rostedt

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Sat, 2009-09-12 at 16:42 +0530, Jaswinder Singh Rajput wrote:

>
> Here are some more trace related warnings in current linus (as well as
> -tip) tree :
>
> CHECK arch/x86/kernel/ptrace.c
> include/trace/events/syscalls.h:18:1: warning: symbol 'ftrace_raw_output_sys_enter' was not declared. Should it be static?
> include/trace/events/syscalls.h:42:1: warning: symbol 'ftrace_raw_output_sys_exit' was not declared. Should it be static?
> include/trace/events/syscalls.h:18:1: warning: symbol 'ftrace_define_fields_sys_enter' was not declared. Should it be static?
> include/trace/events/syscalls.h:42:1: warning: symbol 'ftrace_define_fields_sys_exit' was not declared. Should it be static?

I just wrote a patch to fix the above.

> include/trace/events/syscalls.h:18:1: error: bad constant expression
> include/trace/events/syscalls.h:42:1: error: bad constant expression

Not sure why sparse is failing on this. Looking at the sched.c code, I
ran "make kernel/sched.i" and then removed the CPP expressions and then
expanded the macros and here's where it is failing:

static void ftrace_profile_sched_kthread_stop(struct task_struct *t)
{
struct ftrace_data_offsets_sched_kthread_stop __attribute__((unused)) __data_offsets;
struct ftrace_event_call *event_call = &event_sched_kthread_stop;
extern void perf_tpcounter_event(int, u64, u64, void *, int);
struct ftrace_raw_sched_kthread_stop *entry;
u64 __addr = 0, __count = 1;
unsigned long irq_flags;

int __entry_size;
int __data_size;
int pc;

do { ({ unsigned long __dummy;
typeof(irq_flags) __dummy2;
(void)(&__dummy == &__dummy2);
1;
});
do { (irq_flags) = __raw_local_save_flags();
} while (0);
} while (0);
pc = (current_thread_info()->preempt_count);
__data_size = ftrace_get_offsets_sched_kthread_stop(&__data_offsets, t);
__entry_size = (((__data_size + sizeof(*entry) + sizeof(u32))+((typeof(__data_size + sizeof(*entry) + sizeof(u32)))(sizeof(u64))-1))&~((typeof(__data_size + sizeof(*entry) + sizeof(u32)))(sizeof(u64))-1));
__entry_size -= sizeof(u32);
do {
char raw_data[__entry_size]; <<<<----------- FAILURE HERE
struct trace_entry *ent;
*(u64 *)(&raw_data[__entry_size - sizeof(u64)]) = 0ULL;
entry = (struct ftrace_raw_sched_kthread_stop *)raw_data;
ent = &entry->ent;
tracing_generic_entry_update(ent, irq_flags, pc);
ent->type = event_call->id;
{ memcpy(entry->comm, t->comm, 16);
entry->pid = t->pid;
;
} perf_tpcounter_event(event_call->id, __addr, __count, entry, __entry_size);
} while (0);
};

Sure enough, sparse does not like the __entry_size. I replaced it with
"10" and sparse was happy with it. That is a perfectly legal entry, so
this looks more like a bug with sparse.

I just tested this too:

static void func(int size_me) {
char array[size_me];

memcpy(array, "hello", size);
};

and sparse failed on it as well. Note, you need to have something call
func, or sparse will ignore it.


-- Steve

2009-09-14 17:09:30

by Christopher Li

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <[email protected]> wrote:
> static void func(int size_me) {
> ? ? ? ?char array[size_me];
>
> ? ? ? ?memcpy(array, "hello", size);
> };
>
> and sparse failed on it as well. Note, you need to have something call
> func, or sparse will ignore it.

Gcc allows variable size. Sparse expects the size of an array is constant.
For the kernel using variable array size is consider bad. Because the kernel
has very limited stack size. (8K if I remember correctly). Using dynamic array
is very easy to overflow the stack without realizing it.

It deserves a warning. I agree the warning message can use a better description
though.

Chris

2009-09-14 18:18:51

by Steven Rostedt

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, 2009-09-14 at 10:09 -0700, Christopher Li wrote:
> On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <[email protected]> wrote:
> > static void func(int size_me) {
> > char array[size_me];
> >
> > memcpy(array, "hello", size);
> > };
> >
> > and sparse failed on it as well. Note, you need to have something call
> > func, or sparse will ignore it.
>
> Gcc allows variable size. Sparse expects the size of an array is constant.
> For the kernel using variable array size is consider bad. Because the kernel
> has very limited stack size. (8K if I remember correctly). Using dynamic array
> is very easy to overflow the stack without realizing it.
>
> It deserves a warning. I agree the warning message can use a better description
> though.

Good point!

I've added Frederic to the Cc list, since he wrote the code.

Frederic, how big can one of those events get. The ring buffer (and
TRACE_EVENT) allow up to almost a page size, which is very hefty for the
stack. This code needs to either be rewritten or we need to set a limit
to the size of a profile entry.

We could add:

if (__entry_size > 256)
return;

Thoughts?

-- Steve


2009-09-14 18:25:05

by Peter Zijlstra

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, 2009-09-14 at 14:17 -0400, Steven Rostedt wrote:
> Frederic, how big can one of those events get. The ring buffer (and
> TRACE_EVENT) allow up to almost a page size, which is very hefty for the
> stack. This code needs to either be rewritten or we need to set a limit
> to the size of a profile entry.

Yeah, that needs to get a re-write.. I've complained about this when it
went in.

2009-09-14 18:32:49

by Steven Rostedt

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, 2009-09-14 at 20:23 +0200, Peter Zijlstra wrote:
> On Mon, 2009-09-14 at 14:17 -0400, Steven Rostedt wrote:
> > Frederic, how big can one of those events get. The ring buffer (and
> > TRACE_EVENT) allow up to almost a page size, which is very hefty for the
> > stack. This code needs to either be rewritten or we need to set a limit
> > to the size of a profile entry.
>
> Yeah, that needs to get a re-write.. I've complained about this when it
> went in.

One answer is to create a per cpu buffer that is big enough to hold the
data needed. Then you can disable interrupts an use it without worry.

If you need to also handle NMIs, then create a per_cpu NMI buffer too,
and use that if "in_nmi()" is true.

-- Steve

2009-09-14 18:38:15

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, Sep 14, 2009 at 02:17:18PM -0400, Steven Rostedt wrote:
> On Mon, 2009-09-14 at 10:09 -0700, Christopher Li wrote:
> > On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <[email protected]> wrote:
> > > static void func(int size_me) {
> > > char array[size_me];
> > >
> > > memcpy(array, "hello", size);
> > > };
> > >
> > > and sparse failed on it as well. Note, you need to have something call
> > > func, or sparse will ignore it.
> >
> > Gcc allows variable size. Sparse expects the size of an array is constant.
> > For the kernel using variable array size is consider bad. Because the kernel
> > has very limited stack size. (8K if I remember correctly). Using dynamic array
> > is very easy to overflow the stack without realizing it.
> >
> > It deserves a warning. I agree the warning message can use a better description
> > though.
>
> Good point!
>
> I've added Frederic to the Cc list, since he wrote the code.
>
> Frederic, how big can one of those events get. The ring buffer (and
> TRACE_EVENT) allow up to almost a page size, which is very hefty for the
> stack. This code needs to either be rewritten or we need to set a limit
> to the size of a profile entry.
>
> We could add:
>
> if (__entry_size > 256)
> return;
>
> Thoughts?
>


Well it can be big, especially once we play with array fields or
__string().

I can manage the __string() that said, by only copying their
pointer and later delay the copy.

Well actually I would like to rewrite all that entirely to avoid
any stack allocation, especially for arrays and string.

Lemme think about a CPP magic way to directly interact with perf
buffer. I think it's possible.

2009-09-14 18:41:28

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, Sep 14, 2009 at 02:31:16PM -0400, Steven Rostedt wrote:
> On Mon, 2009-09-14 at 20:23 +0200, Peter Zijlstra wrote:
> > On Mon, 2009-09-14 at 14:17 -0400, Steven Rostedt wrote:
> > > Frederic, how big can one of those events get. The ring buffer (and
> > > TRACE_EVENT) allow up to almost a page size, which is very hefty for the
> > > stack. This code needs to either be rewritten or we need to set a limit
> > > to the size of a profile entry.
> >
> > Yeah, that needs to get a re-write.. I've complained about this when it
> > went in.
>
> One answer is to create a per cpu buffer that is big enough to hold the
> data needed. Then you can disable interrupts an use it without worry.
>
> If you need to also handle NMIs, then create a per_cpu NMI buffer too,
> and use that if "in_nmi()" is true.
>
> -- Steve


Looks like a nice idea.

Peter, does that sound acceptable to you to disable interrupts during a
profiled tracepoint event?

2009-09-14 22:36:10

by Stephen Rothwell

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

Hi Steve,

On Mon, 14 Sep 2009 09:50:01 -0400 Steven Rostedt <[email protected]> wrote:
>
> On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> > * Stephen Rothwell <[email protected]> wrote:
>
> > > Now that this is in Linus' tree, can we have a fix for the waning,
> > > please?
> >
> > The first warning got fixed 1.5 months ago - the second one at line
> > 797 is still there but harmless - you can ignore it for now, it will
> > be fixed.
>
> The first warning never got fixed. The typecast was placed in the wrong
> spot (my fault). I sent another patch that fixes both warnings.

Thanks for that.

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (723.00 B)
(No filename) (198.00 B)
Download all attachments

2009-09-15 07:17:22

by Peter Zijlstra

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

On Mon, 2009-09-14 at 20:41 +0200, Frederic Weisbecker wrote:
> On Mon, Sep 14, 2009 at 02:31:16PM -0400, Steven Rostedt wrote:
> > On Mon, 2009-09-14 at 20:23 +0200, Peter Zijlstra wrote:
> > > On Mon, 2009-09-14 at 14:17 -0400, Steven Rostedt wrote:
> > > > Frederic, how big can one of those events get. The ring buffer (and
> > > > TRACE_EVENT) allow up to almost a page size, which is very hefty for the
> > > > stack. This code needs to either be rewritten or we need to set a limit
> > > > to the size of a profile entry.
> > >
> > > Yeah, that needs to get a re-write.. I've complained about this when it
> > > went in.
> >
> > One answer is to create a per cpu buffer that is big enough to hold the
> > data needed. Then you can disable interrupts an use it without worry.
> >
> > If you need to also handle NMIs, then create a per_cpu NMI buffer too,
> > and use that if "in_nmi()" is true.
> >
> > -- Steve
>
>
> Looks like a nice idea.
>
> Peter, does that sound acceptable to you to disable interrupts during a
> profiled tracepoint event?

It does anyway, a little further down the line, so sure.

2009-09-15 09:01:20

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

2009/9/15, Peter Zijlstra <[email protected]>:
> On Mon, 2009-09-14 at 20:41 +0200, Frederic Weisbecker wrote:
>> On Mon, Sep 14, 2009 at 02:31:16PM -0400, Steven Rostedt wrote:
>> > On Mon, 2009-09-14 at 20:23 +0200, Peter Zijlstra wrote:
>> > > On Mon, 2009-09-14 at 14:17 -0400, Steven Rostedt wrote:
>> > > > Frederic, how big can one of those events get. The ring buffer (and
>> > > > TRACE_EVENT) allow up to almost a page size, which is very hefty for
>> > > > the
>> > > > stack. This code needs to either be rewritten or we need to set a
>> > > > limit
>> > > > to the size of a profile entry.
>> > >
>> > > Yeah, that needs to get a re-write.. I've complained about this when
>> > > it
>> > > went in.
>> >
>> > One answer is to create a per cpu buffer that is big enough to hold the
>> > data needed. Then you can disable interrupts an use it without worry.
>> >
>> > If you need to also handle NMIs, then create a per_cpu NMI buffer too,
>> > and use that if "in_nmi()" is true.
>> >
>> > -- Steve
>>
>>
>> Looks like a nice idea.
>>
>> Peter, does that sound acceptable to you to disable interrupts during a
>> profiled tracepoint event?
>
> It does anyway, a little further down the line, so sure.


Ok I'll fix it soon using Steve's idea then.

Thanks.