linux-next: tip tree build warning

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

Hi all,

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

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: tip tree build warning

[Steven Rostedt]

On Tue, 2009-08-04 at 16:16 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> 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.

-- Steve

Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

Hi Steve,

On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <srostedt@redhat.com> 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                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Steve,
> 
> On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]

Hi Ingo,

On Sat, 12 Sep 2009 09:39:06 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <srostedt@redhat.com> 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                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Jaswinder Singh Rajput]

On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi Steve,
> > 
> > On Tue, 04 Aug 2009 12:24:49 -0400 Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> * Stephen Rothwell <sfr@canb.auug.org.au> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Ingo Molnar]

* Steven Rostedt <srostedt@redhat.com> wrote:

> On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> > * Stephen Rothwell <sfr@canb.auug.org.au> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Christopher Li]

On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Christopher Li]

On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

On Mon, 2009-09-14 at 10:09 -0700, Christopher Li wrote:
> On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

On Mon, 2009-09-14 at 10:09 -0700, Christopher Li wrote:
> On Mon, Sep 14, 2009 at 8:16 AM, Steven Rostedt <srostedt@redhat.com> 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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Peter Zijlstra]

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.

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Peter Zijlstra]

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.

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Steven Rostedt]

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

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Frederic Weisbecker]

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 <srostedt@redhat.com> 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.

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Frederic Weisbecker]

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?

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

Hi Steve,

On Mon, 14 Sep 2009 09:50:01 -0400 Steven Rostedt <srostedt@redhat.com> wrote:
>
> On Sat, 2009-09-12 at 09:39 +0200, Ingo Molnar wrote:
> > * Stephen Rothwell <sfr@canb.auug.org.au> 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                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Peter Zijlstra]

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.

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Frédéric Weisbecker]

2009/9/15, Peter Zijlstra <peterz@infradead.org>:
> 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.

Re: Warning from ring buffer code (Was: Re: linux-next: tip tree build warning)

[Frédéric Weisbecker]

2009/9/15, Peter Zijlstra <peterz@infradead.org>:
> 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.

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

Hi all,

Today's linux-next build (powerpc allyesconfig) produced this warning:

kernel/sched.c:1636: warning: 'update_shares_locked' defined but not used

Introduced by commit f492e12ef050e02bf0185b6b57874992591b9be1 ("sched:
Remove load_balance_newidle()").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

Hi all,

Today's linux-next build (powerpc allnoconfig) produced this warning:

kernel/sched.c: In function 'wake_up_new_task':
kernel/sched.c:2631: warning: unused variable 'cpu'

Introduced by commit fabf318e5e4bda0aca2b0d617b191884fda62703 ("sched:
Fix fork vs hotplug vs cpuset namespaces").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

Hi all,

Today's (and the previous few) linux-next build (powerpc allyesconfig)
produced this warning:

lib/dma-debug.c: In function 'dma_debug_device_change':
lib/dma-debug.c:680: warning: 'return' with no value, in function returning non-void

Introduced by commit f797d9881b62c2ddb1d2e7bd80d87141949c84aa
("dma-debug: Do not add notifier when dma debugging is disabled").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

Hi all,

Today's linux-next build (powerpc allyesconfig) produced this warning:

In file included from kernel/trace/trace.h:14,
                 from kernel/trace/trace_selftest_dynamic.c:1:
include/linux/hw_breakpoint.h: In function 'modify_user_hw_breakpoint':
include/linux/hw_breakpoint.h:96: warning: return makes integer from pointer without a cast

Introduced by commit 44234adcdce38f83c56e05f808ce656175b4beeb
("hw-breakpoints: Modify breakpoints without unregistering them") from
the tip tree.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Frederic Weisbecker]

On Wed, Nov 25, 2009 at 09:53:38PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (powerpc ppc64_defconfig) produced this warning:
> 
> kernel/perf_event.c:4306: warning: 'bp_perf_event_destroy' defined but not used
> 
> Introduced by commit 24f1e32c60c45c89a997c73395b69c8af6f0a84e
> ("hw-breakpoints: Rewrite the hw-breakpoints layer on top of perf
> events").



Oh I see, it's not used in the off-case. I'll fix that soon.
Thanks for the report!

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Hi all,

Today's linux-next build (powerpc ppc64_defconfig) produced this warning:

kernel/perf_event.c:4306: warning: 'bp_perf_event_destroy' defined but not used

Introduced by commit 24f1e32c60c45c89a997c73395b69c8af6f0a84e
("hw-breakpoints: Rewrite the hw-breakpoints layer on top of perf
events").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Randy Dunlap]

On Mon, 16 Nov 2009 09:05:07 -0800 Randy Dunlap wrote:

> On Mon, 16 Nov 2009 16:25:03 +1100 Stephen Rothwell wrote:
> 
> > Hi all,
> > 
> > Today's linux-next build (x86_64 allmodconfig) produced this warning:
> > 
> > arch/x86/kernel/apic/apic.c: In function 'calibrate_APIC_clock':
> > arch/x86/kernel/apic/apic.c:650: warning: format '%ld' expects type 'long int', but argument 2 has type 'u32'
> > 
> > Introduced by commit 23af368e9a904f59256c27d371ce223d6cee0430
> > ("clockevents: Use u32 for mult and shift factors").
> 
> 
> also this warning:
> 
> arch/x86/kernel/vmiclock_32.c:230: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'u32'
> 
> (i386 build)


Thomas posted a patch for this, but I'm still seeing this warning (20091123),
so I guess his patch hasn't been merged/pushed yet?

thanks,
---
~Randy

Re: linux-next: tip tree build warning

[Randy Dunlap]

On Mon, 16 Nov 2009 16:25:03 +1100 Stephen Rothwell wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) produced this warning:
> 
> arch/x86/kernel/apic/apic.c: In function 'calibrate_APIC_clock':
> arch/x86/kernel/apic/apic.c:650: warning: format '%ld' expects type 'long int', but argument 2 has type 'u32'
> 
> Introduced by commit 23af368e9a904f59256c27d371ce223d6cee0430
> ("clockevents: Use u32 for mult and shift factors").


also this warning:

arch/x86/kernel/vmiclock_32.c:230: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'u32'

(i386 build)

---
~Randy

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 489 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) produced this warning:

arch/x86/kernel/apic/apic.c: In function 'calibrate_APIC_clock':
arch/x86/kernel/apic/apic.c:650: warning: format '%ld' expects type 'long int', but argument 2 has type 'u32'

Introduced by commit 23af368e9a904f59256c27d371ce223d6cee0430
("clockevents: Use u32 for mult and shift factors").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> +++ linux-2.6.git/arch/x86/include/asm/apic.h
> @@ -310,7 +310,7 @@ struct apic {
>  	int (*apicid_to_node)(int logical_apicid);
>  	int (*cpu_to_logical_apicid)(int cpu);
>  	int (*cpu_present_to_apicid)(int mps_cpu);
> -	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
> +	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *map);

Yep, passing masks by reference is unconditonal goodness - that's the 
cleanup we want.

Mind sending a delta patch against tip:master?

	Ingo

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Hi Cyril,

On Sun, 8 Nov 2009 16:16:45 +0300 Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
> (Btw, Stephen could you CC me next time if you get commit id
>  with me in authors, so I wouldn't miss problem).

Yeah, sorry about that - at the time I didn't know if this was caused
directly by your commit or was just a side effect of some some other tree.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
| 
| * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| 
| > Hi all,
| > 
| > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| > >
| > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
| > > {
| > >         return physid_mask_of_physid(phys_apicid);
| > > }
| > 
| > I just noticed that this function (default_apicid_to_cpu_present) is 
| > declared "static inline in a header" but looks like it is only used by 
| > assigning its address to a function pointer.  Its only use for x86_64 
| > is in arch/x86/kernel/apic/apic_noop.c ...
| 
| yes, that might be a real problem - returning the mask like that is 
| messy. Thanks, will check.
| 
| 	Ingo
|

ok, here is what I've just cooked. Please review, I've CC'ed a few
"knowing-apic-code-quite-well" persons just to be sure.

Note that we still have a few items in "struct apic" which
operate with physid_mask_t on stack.

I think perhaps it's a good idea to touch this data via pointers
povided by caller:

	physid_mask_t (*ioapic_phys_id_map)(physid_mask_t map);

I didn't manage to cover it today -- will do tomorrow if patch
approach would be approved.

Also, it's just warning (yet) since we don't use those routines
in x86-64 so it doesn't harm.

Anyway, please review, comment, complain and etc... would appreciate.

	-- Cyrill

p.s.: i don't have inet access in office so will be able to reply
at tomorrow evening only.
---
x86,apic: Do not use stack'ed physid_mask_t in apicid_to_cpu_present

Stephen Rothwell pointed out that apic-noop (when gets compiled
in x86-84 environment) potentially may consume too much stack space.

|
| Hi all,
|
| Today's linux-next build (x86_64 allmodconfig) produced this warning:
|
| In file included from arch/x86/include/asm/smp.h:13,
|                 from arch/x86/include/asm/mmzone_64.h:12,
|                 from arch/x86/include/asm/mmzone.h:4,
|                 from include/linux/mmzone.h:783,
|                 from include/linux/gfp.h:4,
|                 from include/linux/kmod.h:22,
|                 from include/linux/module.h:13,
|                 from arch/x86/kernel/apic/apic_noop.c:14:
| arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
| arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes
|
| It may not have been caused by the tip tree, but I can't find what
| changed to cause this and a commit from the tip tree has exposed it
| (9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the NOOP
| apic driver").
|

So I would say this is a bug in apic-noop (in fact we don't use
default_apicid_to_cpu_present if operate in 64bit mode but it's a sign
that something is wrong with code design). The key problem is that
physid_mask_t is an array with a size depending on MAX_APICS, which
in turn is big enough on x86-64 to trigger compiler warning.

So to prevent such a situation in future we should use physid_mask_t
pointer leaving apic driver with a task to operate over data but not
allocate it. Caller should instead.

This allow us throw out some code as well since physid_set_mask_of_physid
already implement the functionality we need.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/include/asm/apic.h      |    7 +------
 arch/x86/kernel/apic/apic_noop.c |    2 +-
 arch/x86/kernel/apic/bigsmp_32.c |    7 +------
 arch/x86/kernel/apic/es7000_32.c |    8 ++------
 arch/x86/kernel/apic/io_apic.c   |    4 ++--
 arch/x86/kernel/apic/numaq_32.c  |    4 ++--
 arch/x86/kernel/apic/probe_32.c  |    2 +-
 arch/x86/kernel/apic/summit_32.c |    4 ++--
 arch/x86/kernel/visws_quirks.c   |    2 +-
 9 files changed, 13 insertions(+), 27 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/apic.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/apic.h
+++ linux-2.6.git/arch/x86/include/asm/apic.h
@@ -310,7 +310,7 @@ struct apic {
 	int (*apicid_to_node)(int logical_apicid);
 	int (*cpu_to_logical_apicid)(int cpu);
 	int (*cpu_present_to_apicid)(int mps_cpu);
-	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
+	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *map);
 	void (*setup_portio_remap)(void);
 	int (*check_phys_apicid_present)(int phys_apicid);
 	void (*enable_apic_mode)(void);
@@ -585,11 +585,6 @@ extern int default_cpu_present_to_apicid
 extern int default_check_phys_apicid_present(int phys_apicid);
 #endif
 
-static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_32
Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,7 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
Index: linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/bigsmp_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
@@ -93,11 +93,6 @@ static int bigsmp_cpu_present_to_apicid(
 	return BAD_APICID;
 }
 
-static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 /* Mapping from cpu number to logical apicid */
 static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 {
@@ -230,7 +225,7 @@ struct apic apic_bigsmp = {
 	.apicid_to_node			= bigsmp_apicid_to_node,
 	.cpu_to_logical_apicid		= bigsmp_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= bigsmp_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= bigsmp_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= bigsmp_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/es7000_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/es7000_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/es7000_32.c
@@ -539,14 +539,10 @@ static int es7000_cpu_present_to_apicid(
 
 static int cpu_id;
 
-static physid_mask_t es7000_apicid_to_cpu_present(int phys_apicid)
+static void es7000_apicid_to_cpu_present(int phys_apicid, physid_mask_t *map)
 {
-	physid_mask_t mask;
-
-	mask = physid_mask_of_physid(cpu_id);
+	physid_set_mask_of_physid(cpu_id, map);
 	++cpu_id;
-
-	return mask;
 }
 
 /* Mapping from cpu number to logical apicid */
Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
@@ -2073,7 +2073,7 @@ void __init setup_ioapic_ids_from_mpc(vo
 			mp_ioapics[apic_id].apicid = i;
 		} else {
 			physid_mask_t tmp;
-			tmp = apic->apicid_to_cpu_present(mp_ioapics[apic_id].apicid);
+			apic->apicid_to_cpu_present(mp_ioapics[apic_id].apicid, &tmp);
 			apic_printk(APIC_VERBOSE, "Setting %d in the "
 					"phys_id_present_map\n",
 					mp_ioapics[apic_id].apicid);
@@ -3969,7 +3969,7 @@ int __init io_apic_get_unique_id(int ioa
 		apic_id = i;
 	}
 
-	tmp = apic->apicid_to_cpu_present(apic_id);
+	apic->apicid_to_cpu_present(apic_id, &tmp);
 	physids_or(apic_id_map, apic_id_map, tmp);
 
 	if (reg_00.bits.ID != apic_id) {
Index: linux-2.6.git/arch/x86/kernel/apic/numaq_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/numaq_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/numaq_32.c
@@ -402,12 +402,12 @@ static inline int numaq_apicid_to_node(i
 	return logical_apicid >> 4;
 }
 
-static inline physid_mask_t numaq_apicid_to_cpu_present(int logical_apicid)
+static void numaq_apicid_to_cpu_present(int logical_apicid, physid_mask_t *map)
 {
 	int node = numaq_apicid_to_node(logical_apicid);
 	int cpu = __ffs(logical_apicid & 0xf);
 
-	return physid_mask_of_physid(cpu + 4*node);
+	physid_set_mask_of_physid(cpu + 4*node, map);
 }
 
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
Index: linux-2.6.git/arch/x86/kernel/apic/probe_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/probe_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/probe_32.c
@@ -108,7 +108,7 @@ struct apic apic_default = {
 	.apicid_to_node			= default_apicid_to_node,
 	.cpu_to_logical_apicid		= default_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/summit_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/summit_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/summit_32.c
@@ -267,9 +267,9 @@ static physid_mask_t summit_ioapic_phys_
 	return physids_promote(0x0F);
 }
 
-static physid_mask_t summit_apicid_to_cpu_present(int apicid)
+static void summit_apicid_to_cpu_present(int apicid, physid_mask_t *map)
 {
-	return physid_mask_of_physid(0);
+	physid_set_mask_of_physid(0, map);
 }
 
 static int summit_check_phys_apicid_present(int physical_apicid)
Index: linux-2.6.git/arch/x86/kernel/visws_quirks.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/visws_quirks.c
+++ linux-2.6.git/arch/x86/kernel/visws_quirks.c
@@ -183,7 +183,7 @@ static void __init MP_processor_info(str
 		return;
 	}
 
-	apic_cpus = apic->apicid_to_cpu_present(m->apicid);
+	apic->apicid_to_cpu_present(m->apicid, &apic_cpus);
 	physids_or(phys_cpu_present_map, phys_cpu_present_map, apic_cpus);
 	/*
 	 * Validate version

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Nov 08, 2009 at 02:32:47PM +0100]
| 
...
| > +
| > +#ifdef CONFIG_X86_32
| >  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
| > +#else
| > +	.apicid_to_cpu_present		= NULL,
| > +#endif
| 
| would be better to unify this instead ...
| 
| 	Ingo
| 

[not for inclusion]

Here is what I'm going to implement (it is not finished yet,
but just to show the idea -- ie to get rid of physid_mask_t
passed as an argument at all but use pointers instead since
callers already have bitmask allocated).

And physid_set_mask_of_physid already do the work for us.
Hmm?

(Ingo, I think you may apply the former patch just to
 have the issue fixed this way temporary)

	-- Cyrill
---
 arch/x86/include/asm/apic.h      |    7 +------
 arch/x86/kernel/apic/apic_noop.c |    2 +-
 arch/x86/kernel/apic/bigsmp_32.c |    7 +------
 arch/x86/kernel/apic/probe_32.c  |    2 +-
 4 files changed, 4 insertions(+), 14 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/apic.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/apic.h
+++ linux-2.6.git/arch/x86/include/asm/apic.h
@@ -310,7 +310,7 @@ struct apic {
 	int (*apicid_to_node)(int logical_apicid);
 	int (*cpu_to_logical_apicid)(int cpu);
 	int (*cpu_present_to_apicid)(int mps_cpu);
-	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
+	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *bitmap);
 	void (*setup_portio_remap)(void);
 	int (*check_phys_apicid_present)(int phys_apicid);
 	void (*enable_apic_mode)(void);
@@ -585,11 +585,6 @@ extern int default_cpu_present_to_apicid
 extern int default_check_phys_apicid_present(int phys_apicid);
 #endif
 
-static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_32
Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,7 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
Index: linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/bigsmp_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
@@ -93,11 +93,6 @@ static int bigsmp_cpu_present_to_apicid(
 	return BAD_APICID;
 }
 
-static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 /* Mapping from cpu number to logical apicid */
 static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 {
@@ -230,7 +225,7 @@ struct apic apic_bigsmp = {
 	.apicid_to_node			= bigsmp_apicid_to_node,
 	.cpu_to_logical_apicid		= bigsmp_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= bigsmp_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= bigsmp_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= bigsmp_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/probe_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/probe_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/probe_32.c
@@ -108,7 +108,7 @@ struct apic apic_default = {
 	.apicid_to_node			= default_apicid_to_node,
 	.cpu_to_logical_apicid		= default_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Nov 08, 2009 at 02:32:47PM +0100]
...
| 
| > +
| > +#ifdef CONFIG_X86_32
| >  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
| > +#else
| > +	.apicid_to_cpu_present		= NULL,
| > +#endif
| 
| would be better to unify this instead ...
| 
| 	Ingo
| 

OK, wait a bit.

	-- Cyrill

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> [Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
> | 
> | * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> | 
> | > Hi all,
> | > 
> | > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> | > >
> | > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> | > > {
> | > >         return physid_mask_of_physid(phys_apicid);
> | > > }
> | > 
> | > I just noticed that this function (default_apicid_to_cpu_present) is 
> | > declared "static inline in a header" but looks like it is only used by 
> | > assigning its address to a function pointer.  Its only use for x86_64 
> | > is in arch/x86/kernel/apic/apic_noop.c ...
> | 
> | yes, that might be a real problem - returning the mask like that is 
> | messy. Thanks, will check.
> | 
> | 	Ingo
> | 
> 
> Darn, my fault sorry! Here is an update which fixes the issue.
> (Btw, Stephen could you CC me next time if you get commit id
>  with me in authors, so I wouldn't miss problem).
> 
> Please review, comments/complains are quite welcome!
> 
> 	-- Cyrill
> ---
> x86,apic: Get rid of apicid_to_cpu_present assign on X86-64
> 
> In fact it's never get used on x86-64 (for 64 bit platform
> we use differ technique to enumerate io-units).
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>  arch/x86/kernel/apic/apic_noop.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
> +++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
> @@ -162,7 +162,12 @@ struct apic apic_noop = {
>  
>  	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
>  	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
> +
> +#ifdef CONFIG_X86_32
>  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
> +#else
> +	.apicid_to_cpu_present		= NULL,
> +#endif

would be better to unify this instead ...

	Ingo

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
| 
| * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| 
| > Hi all,
| > 
| > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| > >
| > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
| > > {
| > >         return physid_mask_of_physid(phys_apicid);
| > > }
| > 
| > I just noticed that this function (default_apicid_to_cpu_present) is 
| > declared "static inline in a header" but looks like it is only used by 
| > assigning its address to a function pointer.  Its only use for x86_64 
| > is in arch/x86/kernel/apic/apic_noop.c ...
| 
| yes, that might be a real problem - returning the mask like that is 
| messy. Thanks, will check.
| 
| 	Ingo
| 

Darn, my fault sorry! Here is an update which fixes the issue.
(Btw, Stephen could you CC me next time if you get commit id
 with me in authors, so I wouldn't miss problem).

Please review, comments/complains are quite welcome!

	-- Cyrill
---
x86,apic: Get rid of apicid_to_cpu_present assign on X86-64

In fact it's never get used on x86-64 (for 64 bit platform
we use differ technique to enumerate io-units).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/apic/apic_noop.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,12 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
+
+#ifdef CONFIG_X86_32
 	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+#else
+	.apicid_to_cpu_present		= NULL,
+#endif
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> > {
> >         return physid_mask_of_physid(phys_apicid);
> > }
> 
> I just noticed that this function (default_apicid_to_cpu_present) is 
> declared "static inline in a header" but looks like it is only used by 
> assigning its address to a function pointer.  Its only use for x86_64 
> is in arch/x86/kernel/apic/apic_noop.c ...

yes, that might be a real problem - returning the mask like that is 
messy. Thanks, will check.

	Ingo

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Hi all,

On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> {
>         return physid_mask_of_physid(phys_apicid);
> }

I just noticed that this function (default_apicid_to_cpu_present) is
declared "static inline in a header" but looks like it is only used by
assigning its address to a function pointer.  Its only use for x86_64 is
in arch/x86/kernel/apic/apic_noop.c ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

Hi Ingo,

On Wed, 28 Oct 2009 08:31:45 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> 
> That commit is very simple. Are you sure it's not GCC bogosity?

That commit just produces the warning (since it introduced the
apic_noop.c file) and probably has nothing directly to do with the
problem (as I said).

The relevant part of apic.h is:

static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
{
        return physid_mask_of_physid(phys_apicid);
}

Where physid_mask_of_physid() manipulates a physid_mask_t which contains
an array of unsigned longs that is BITS_TO_LONGS(MAX_APICS) long and
MAX_APICS is 32768.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) produced this warning:
> 
> In file included from arch/x86/include/asm/smp.h:13,
>                  from arch/x86/include/asm/mmzone_64.h:12,
>                  from arch/x86/include/asm/mmzone.h:4,
>                  from include/linux/mmzone.h:783,
>                  from include/linux/gfp.h:4,
>                  from include/linux/kmod.h:22,
>                  from include/linux/module.h:13,
>                  from arch/x86/kernel/apic/apic_noop.c:14:
> arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
> arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes
> 
> It may not have been caused by the tip tree, but I can't find what 
> changed to cause this and a commit from the tip tree has exposed it 
> (9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the 
> NOOP apic driver").

That commit is very simple. Are you sure it's not GCC bogosity?

	Ingo

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) produced this warning:

In file included from arch/x86/include/asm/smp.h:13,
                 from arch/x86/include/asm/mmzone_64.h:12,
                 from arch/x86/include/asm/mmzone.h:4,
                 from include/linux/mmzone.h:783,
                 from include/linux/gfp.h:4,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from arch/x86/kernel/apic/apic_noop.c:14:
arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes

It may not have been caused by the tip tree, but I can't find what
changed to cause this and a commit from the tip tree has exposed it
(9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the NOOP
apic driver").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) produced this warning:

arch/x86/kernel/cpu/perf_event.c: In function 'intel_get_event_idx':
arch/x86/kernel/cpu/perf_event.c:1445: warning: 'event_constraint' is used uninitialized in this function

Introduced by commit b690081d4d3f6a23541493f1682835c3cd5c54a1
("perf_events: Add event constraints support for Intel processors").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Geert Uytterhoeven]

On Fri, Sep 11, 2009 at 10:56, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> Today's linux-next build (powerpc allnoconfig) produced this warning:
>
> kernel/sched.c:122: warning: 'double_rq_lock' declared 'static' but never defined
>
> Introduced by commit 18a3885fc1ffa92c2212ff0afdf033403d5b0fa0 ("sched:
> Remove reciprocal for cpu_power").  This is a build with CONFIG_SMP
> undefined.

Yep. same here.

It's even more fishy:
  - double_rq_lock() is useded inside #ifdef CONFIG_PREEMPT
  - double_rq_lock() is defined inside #ifdef CONFIG_SMP
  - double_rq_lock() is used inside #ifdef CONFIG_HOTPLUG_CPU

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Hi all,

Today's linux-next build (powerpc allnoconfig) produced this warning:

kernel/sched.c:122: warning: 'double_rq_lock' declared 'static' but never defined

Introduced by commit 18a3885fc1ffa92c2212ff0afdf033403d5b0fa0 ("sched:
Remove reciprocal for cpu_power").  This is a build with CONFIG_SMP
undefined.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]