linux-next: manual merge of the tip tree with the kgdb tree

linux-next: manual merge of the tip tree with the kgdb tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in
kernel/trace/trace.c between commit
d304af88a0105ff5b64cffc9108636ecad1fdd78 ("ftrace,kdb: Extend kdb to be
able to dump the ftrace buffer") from the kgdb tree and commit
7e53bd42d14c75192b99674c40fcc359392da59d ("tracing: Consolidate
protection of reader access to the ring buffer") from the tip tree.

Just context changes (I think).  I fixed it up (see below) and can carry
the fix as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc kernel/trace/trace.c
index b3c786a,5314c90..0000000
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@@ -100,11 -101,11 +101,8 @@@ static inline void ftrace_enable_cpu(vo
  	preempt_enable();
  }
  
 -static cpumask_var_t __read_mostly	tracing_buffer_mask;
 -
 -#define for_each_tracing_cpu(cpu)	\
 -	for_each_cpu(cpu, tracing_buffer_mask)
 +cpumask_var_t __read_mostly	tracing_buffer_mask;
  
- /* Define which cpu buffers are currently read in trace_pipe */
- static cpumask_var_t			tracing_reader_cpumask;
- 
  /*
   * ftrace_dump_on_oops - variable to dump ftrace buffer on oops
   *
@@@ -3066,7 -3131,8 +3123,8 @@@ waitagain
  	iter->pos = -1;
  
  	trace_event_read_lock();
+ 	trace_access_lock(iter->cpu_file);
 -	while (find_next_entry_inc(iter) != NULL) {
 +	while (trace_find_next_entry_inc(iter) != NULL) {
  		enum print_line_t ret;
  		int len = iter->seq.len;
  

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Ingo Molnar]

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

> Hi all,
> 
> Today's linux-next merge of the tip tree got a conflict in
> kernel/trace/trace.c between commit
> d304af88a0105ff5b64cffc9108636ecad1fdd78 ("ftrace,kdb: Extend kdb to be
> able to dump the ftrace buffer") from the kgdb tree and commit
> 7e53bd42d14c75192b99674c40fcc359392da59d ("tracing: Consolidate
> protection of reader access to the ring buffer") from the tip tree.

Hm, Jason, what is that large commit to kernel/trace/ doing in the KGDB tree, 
without any apparent acks from the affected people?

I dont see it anywhere on lkml nor in my mbox. Please submit it to the 
affected maintainers - for the Cc: line see the output of:

  scripts/get_maintainer.pl -f kernel/trace/trace.c

Thanks,

	Ingo

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Jason Wessel]

Ingo Molnar wrote:
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
>   
>> Hi all,
>>
>> Today's linux-next merge of the tip tree got a conflict in
>> kernel/trace/trace.c between commit
>> d304af88a0105ff5b64cffc9108636ecad1fdd78 ("ftrace,kdb: Extend kdb to be
>> able to dump the ftrace buffer") from the kgdb tree and commit
>> 7e53bd42d14c75192b99674c40fcc359392da59d ("tracing: Consolidate
>> protection of reader access to the ring buffer") from the tip tree.
>>     
>
> Hm, Jason, what is that large commit to kernel/trace/ doing in the KGDB tree, 
> without any apparent acks from the affected people?
>   

I had been corresponding with Steven Rostedt directly.  This is actually
the 3rd iteration of the patch (the first two never got checked in
anywhere) and there is still an outstanding question, which I will
inline at the bottom of this email.  The ftdump patch is at the very end
of the kdb series, because this patch will get nuked if Steven or anyone
else has a problem with it.

As for what the patch does, it is routine for dumping the ftrace buffer
while in the kernel debugger context.

> I dont see it anywhere on lkml nor in my mbox. Please submit it to the 
> affected maintainers - for the Cc: line see the output of:
>   

The v2 version of the kdb  series was supposed to go out yesterday
morning (also known as kdb_prototype11).   The new patch is included in
the post, look for: "[PATCH 40/40] ftrace,kdb: Extend kdb to be able to
dump the ftrace buffer"

-- prior inlined correspondence --

Steven Rostedt wrote:

> > On Tue, 2010-01-05 at 23:57 -0600, Jason Wessel wrote:
>   
>> >> Here is another try at adding a dump function for kdb.  I had to
>> >> changes some of the static -> global scope in kernel/trace/trace.c in
>> >> order to be able to reference other semi-private via "trace.h".
>>     
> > 
> > Actually, could you write access functions instead. If we make these
> > items global in scope, then others will just start accessing them
> > directly. I've had this issue before because others have tried to make
> > the global_trace visible by all. But that variable may disappear and
> > break all that use it.
> > 
>   

Thanks for the insight.

Here is v3.

I added a function called trace_init_global_iter().  I'll rename it if
you like.  I also changed the ftrace_dump() to make use of it as well
so we are more likely to see an issue when it changes if there are
more consumers of the function.

The other question it brings up is if you want a helper function for
the atomic_inc / atomic_dec of the tracing cpus.  That would move that
for_each_tracing_cpu macro back into trace.c.

Thanks,
Jason.

[..patch clipped..]

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Steven Rostedt]

On Thu, 2010-01-14 at 09:01 -0600, Jason Wessel wrote:
> Ingo Molnar wrote:
> > * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> >   
> >> Hi all,
> >>
> >> Today's linux-next merge of the tip tree got a conflict in
> >> kernel/trace/trace.c between commit
> >> d304af88a0105ff5b64cffc9108636ecad1fdd78 ("ftrace,kdb: Extend kdb to be
> >> able to dump the ftrace buffer") from the kgdb tree and commit
> >> 7e53bd42d14c75192b99674c40fcc359392da59d ("tracing: Consolidate
> >> protection of reader access to the ring buffer") from the tip tree.
> >>     
> >
> > Hm, Jason, what is that large commit to kernel/trace/ doing in the KGDB tree, 
> > without any apparent acks from the affected people?
> >   
> 
> I had been corresponding with Steven Rostedt directly.  This is actually
> the 3rd iteration of the patch (the first two never got checked in
> anywhere) and there is still an outstanding question, which I will
> inline at the bottom of this email.  The ftdump patch is at the very end
> of the kdb series, because this patch will get nuked if Steven or anyone
> else has a problem with it.

Yep, we've been talking, I just didn't know it went into a public git
tree yet :-)

> 
> As for what the patch does, it is routine for dumping the ftrace buffer
> while in the kernel debugger context.
> 
> > I dont see it anywhere on lkml nor in my mbox. Please submit it to the 
> > affected maintainers - for the Cc: line see the output of:
> >   
> 
> The v2 version of the kdb  series was supposed to go out yesterday
> morning (also known as kdb_prototype11).   The new patch is included in
> the post, look for: "[PATCH 40/40] ftrace,kdb: Extend kdb to be able to
> dump the ftrace buffer"
> 
> -- prior inlined correspondence --
> 
> Steven Rostedt wrote:
> 
> > > On Tue, 2010-01-05 at 23:57 -0600, Jason Wessel wrote:
> >   
> >> >> Here is another try at adding a dump function for kdb.  I had to
> >> >> changes some of the static -> global scope in kernel/trace/trace.c in
> >> >> order to be able to reference other semi-private via "trace.h".
> >>     
> > > 
> > > Actually, could you write access functions instead. If we make these
> > > items global in scope, then others will just start accessing them
> > > directly. I've had this issue before because others have tried to make
> > > the global_trace visible by all. But that variable may disappear and
> > > break all that use it.
> > > 
> >   
> 
> Thanks for the insight.
> 
> Here is v3.
> 
> I added a function called trace_init_global_iter().  I'll rename it if
> you like.  I also changed the ftrace_dump() to make use of it as well
> so we are more likely to see an issue when it changes if there are
> more consumers of the function.
> 
> The other question it brings up is if you want a helper function for
> the atomic_inc / atomic_dec of the tracing cpus.  That would move that
> for_each_tracing_cpu macro back into trace.c.


I think the point Ingo is making, is that the changes to the
kernel/trace directory would best go through the tip tree. This will
ensure that this change does not have any undesirable effects to other
parts of tracing that is being worked on.

There's always an issue with having one subsystem depend on changes in
another subsystem. What I did in the past when PPC needed changes to the
trace directory, was that I made the changes in the trace code directly
against Linus's tree in a separate branch. Then the two git repo's (PPC
and tip) could pull that change in. Which ever one went first to Linus
would get the required change without causing duplicates.

But since this is the last patch in the series, perhaps it can just go
directly into tip?

-- Steve

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Jason Wessel]

Steven Rostedt wrote:
>> Steven Rostedt wrote:
>>
>>     
>>>> On Tue, 2010-01-05 at 23:57 -0600, Jason Wessel wrote:
>>>>         
>>>   
>>>       
>>>>>> Here is another try at adding a dump function for kdb.  I had to
>>>>>> changes some of the static -> global scope in kernel/trace/trace.c in
>>>>>> order to be able to reference other semi-private via "trace.h".
>>>>>>             
>>>>     
>>>>
>>>> Actually, could you write access functions instead. If we make these
>>>> items global in scope, then others will just start accessing them
>>>> directly. I've had this issue before because others have tried to make
>>>> the global_trace visible by all. But that variable may disappear and
>>>> break all that use it.
>>>>
>>>>         
>>>   
>>>       
>> Thanks for the insight.
>>
>> Here is v3.
>>
>> I added a function called trace_init_global_iter().  I'll rename it if
>> you like.  I also changed the ftrace_dump() to make use of it as well
>> so we are more likely to see an issue when it changes if there are
>> more consumers of the function.
>>
>> The other question it brings up is if you want a helper function for
>> the atomic_inc / atomic_dec of the tracing cpus.  That would move that
>> for_each_tracing_cpu macro back into trace.c.
>>     
>
>
> I think the point Ingo is making, is that the changes to the
> kernel/trace directory would best go through the tip tree. This will
> ensure that this change does not have any undesirable effects to other
> parts of tracing that is being worked on.
>
> There's always an issue with having one subsystem depend on changes in
> another subsystem. What I did in the past when PPC needed changes to the
> trace directory, was that I made the changes in the trace code directly
> against Linus's tree in a separate branch. Then the two git repo's (PPC
> and tip) could pull that change in. Which ever one went first to Linus
> would get the required change without causing duplicates.
>
> But since this is the last patch in the series, perhaps it can just go
> directly into tip?
>
>   

It depends if you agree with the changes or not and if this patch is now
in a "ready for merge" state.

The patch is completely self contained and could be merged before or
after the kdb/kgdb change set.  If it is merged into tip or a sub
branch, I just need to know the location of preference.

Some of the kms changes have the same problem as this patch, and they
too might move outside the public kernel debugger git tree.

Thanks,
Jason.

linux-next: manual merge of the tip tree with the kgdb tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in
kernel/Makefile between commit 7d63ecbc5ce463c8ba60e357eafaf47774521f1a
("Move kernel/kgdb.c to kernel/debug/debug_core.c") from the kgdb tree
and commit 58687acba59266735adb8ccd9b5b9aa2c7cd205b ("lockup_detector:
Combine nmi_watchdog and softlockup detector") from the tip tree.

Just context changes.  I fixed it up (see below) and can carry the fix
as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc kernel/Makefile
index 4313d0e,34d123b..0000000
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@@ -68,17 -68,16 +68,17 @@@ obj-$(CONFIG_USER_NS) += user_namespace
  obj-$(CONFIG_PID_NS) += pid_namespace.o
  obj-$(CONFIG_IKCONFIG) += configs.o
  obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
- obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
+ obj-$(CONFIG_SMP) += stop_machine.o
  obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 -obj-$(CONFIG_AUDIT) += audit.o auditfilter.o audit_watch.o
 +obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
  obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
 -obj-$(CONFIG_GCOV_KERNEL) += gcov/
 +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o
  obj-$(CONFIG_AUDIT_TREE) += audit_tree.o
 +obj-$(CONFIG_GCOV_KERNEL) += gcov/
  obj-$(CONFIG_KPROBES) += kprobes.o
 -obj-$(CONFIG_KGDB) += kgdb.o
 +obj-$(CONFIG_KGDB) += debug/
- obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
  obj-$(CONFIG_DETECT_HUNG_TASK) += hung_task.o
+ obj-$(CONFIG_LOCKUP_DETECTOR) += watchdog.o
  obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
  obj-$(CONFIG_SECCOMP) += seccomp.o
  obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Don Zickus]

On Fri, May 14, 2010 at 02:14:06PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tip tree got a conflict in
> kernel/Makefile between commit 7d63ecbc5ce463c8ba60e357eafaf47774521f1a
> ("Move kernel/kgdb.c to kernel/debug/debug_core.c") from the kgdb tree
> and commit 58687acba59266735adb8ccd9b5b9aa2c7cd205b ("lockup_detector:
> Combine nmi_watchdog and softlockup detector") from the tip tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix
> as necessary.

ACK

for the LOCK_DETECTOR and SOFTLOCKUP parts.

Cheers,
Don

> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc kernel/Makefile
> index 4313d0e,34d123b..0000000
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@@ -68,17 -68,16 +68,17 @@@ obj-$(CONFIG_USER_NS) += user_namespace
>   obj-$(CONFIG_PID_NS) += pid_namespace.o
>   obj-$(CONFIG_IKCONFIG) += configs.o
>   obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
> - obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
> + obj-$(CONFIG_SMP) += stop_machine.o
>   obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
>  -obj-$(CONFIG_AUDIT) += audit.o auditfilter.o audit_watch.o
>  +obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
>   obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
>  -obj-$(CONFIG_GCOV_KERNEL) += gcov/
>  +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o
>   obj-$(CONFIG_AUDIT_TREE) += audit_tree.o
>  +obj-$(CONFIG_GCOV_KERNEL) += gcov/
>   obj-$(CONFIG_KPROBES) += kprobes.o
>  -obj-$(CONFIG_KGDB) += kgdb.o
>  +obj-$(CONFIG_KGDB) += debug/
> - obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
>   obj-$(CONFIG_DETECT_HUNG_TASK) += hung_task.o
> + obj-$(CONFIG_LOCKUP_DETECTOR) += watchdog.o
>   obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
>   obj-$(CONFIG_SECCOMP) += seccomp.o
>   obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o

linux-next: manual merge of the tip tree with the kgdb tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tip tree got a conflict in
arch/x86/kernel/kgdb.c between commit
53a2aa296b5108e9bfdcda7f41221e721d9c7474 ("x86,hw_breakpoint,kgdb: kgdb
to use hw_breakpoint API") from the kgdb tree and commit
cc0967490c1c3824bc5b75718b6ca8a51d9f2617 ("x86, hw_breakpoints, kgdb: Fix
kgdb to use hw_breakpoint API") from the tip tree.

These two commits are similar but sufficiently different to make a mess.
I effectively reverted the kgdb tree version and used the tip tree
version.  This also involved reverting the kgdb tree changes to
arch/x86/kernel/hw_breakpoint.c.  I hope I got it right (since I had to
reapply a few other kgdb changes by hand) (an x86_64 allmodconfig build
produces no new warnings or errors, so I have some hope).

Someone should give me some guidance as to a way forward here, 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: linux-next: manual merge of the tip tree with the kgdb tree

[Jason Wessel]

Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/x86/kernel/kgdb.c between commit
> 53a2aa296b5108e9bfdcda7f41221e721d9c7474 ("x86,hw_breakpoint,kgdb: kgdb
> to use hw_breakpoint API") from the kgdb tree and commit
> cc0967490c1c3824bc5b75718b6ca8a51d9f2617 ("x86, hw_breakpoints, kgdb: Fix
> kgdb to use hw_breakpoint API") from the tip tree.
>
> These two commits are similar but sufficiently different to make a mess.
> I effectively reverted the kgdb tree version and used the tip tree
> version.  This also involved reverting the kgdb tree changes to
> arch/x86/kernel/hw_breakpoint.c.  I hope I got it right (since I had to
> reapply a few other kgdb changes by hand) (an x86_64 allmodconfig build
> produces no new warnings or errors, so I have some hope).
>
> Someone should give me some guidance as to a way forward here, please.
>
>   

What is in tip is the correct version. 

It is the age old time dependency of who updates whose tree first, vs
when the linux-next merge runs.  At any rate this is already resolved.

Thanks,
Jason.

Re: linux-next: manual merge of the tip tree with the kgdb tree

[Stephen Rothwell]

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

Hi Jason,

On Mon, 01 Feb 2010 09:46:26 -0600 Jason Wessel <jason.wessel@windriver.com> wrote:
>
> What is in tip is the correct version.

OK, good.

> It is the age old time dependency of who updates whose tree first, vs
> when the linux-next merge runs.  At any rate this is already resolved.

Even better :-)

Thanks.

-- 
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: manual merge of the tip tree with the kgdb tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in
arch/x86/kernel/kgdb.c between commit
1cde88814e04c3b2bbe60e20c0927a0b4e1748ab ("kgdb,i386: Fix corner case
access to sp with NMI watch dog exception") from the kgdb tree and commit
5ca6c0ca5dbf105d7b0ffdae2289519982189730 ("x86: use kernel_stack_pointer
() in kgdb.c") from the tip tree.

I fixed it up (see below) but would like people to check as I am not sure
of the fix.  I can carry this as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc arch/x86/kernel/kgdb.c
index e9afae9,3310d84..0000000
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@@ -85,15 -85,9 +85,15 @@@ void pt_regs_to_gdb_regs(unsigned long 
  	gdb_regs[GDB_DS]	= regs->ds;
  	gdb_regs[GDB_ES]	= regs->es;
  	gdb_regs[GDB_CS]	= regs->cs;
 -	gdb_regs[GDB_SS]	= __KERNEL_DS;
  	gdb_regs[GDB_FS]	= 0xFFFF;
  	gdb_regs[GDB_GS]	= 0xFFFF;
 +	if (user_mode_vm(regs)) {
 +		gdb_regs[GDB_SS] = regs->ss;
 +		gdb_regs[GDB_SP] = regs->sp;
 +	} else {
 +		gdb_regs[GDB_SS] = __KERNEL_DS;
- 		gdb_regs[GDB_SP] = (unsigned long)&regs->sp;
++		gdb_regs[GDB_SP] = kernel_stack_pointer(regs);
 +	}
  #else
  	gdb_regs[GDB_R8]	= regs->r8;
  	gdb_regs[GDB_R9]	= regs->r9;
@@@ -106,8 -100,8 +106,8 @@@
  	gdb_regs32[GDB_PS]	= regs->flags;
  	gdb_regs32[GDB_CS]	= regs->cs;
  	gdb_regs32[GDB_SS]	= regs->ss;
- 	gdb_regs[GDB_SP]	= regs->sp;
 -#endif
+ 	gdb_regs[GDB_SP]	= kernel_stack_pointer(regs);
 +#endif
  }
  
  /**