linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* linux-next: manual merge of the kgdb tree with Linus' tree
@ 2010-08-07  4:05 Stephen Rothwell
  2010-08-07 21:17 ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2010-08-07  4:05 UTC (permalink / raw)
  To: Jason Wessel
  Cc: linux-next, linux-kernel, Mathieu Desnoyers, Paul E. McKenney

Hi Jason,

Today's linux-next merge of the kgdb tree got a conflict in
include/linux/rcupdate.h between commits
551d55a944b143ef26fbd482d1c463199d6f65cf ("tree/tiny rcu: Add debug RCU
head objects") and f5155b33277c9678041a27869165619bb34f722f ("rcu: add an
rcu_dereference_index_check()") from Linus' tree and commit
9e213357d0aeaeb81e213cfd3b9415db5fccc1b5 ("rcu,debug_core: allow the
kernel debugger to reset the rcu stall timer") from the kgdb tree.

Just overlapping additions.  I fixed it up (see below) and can carry the
fix for a while.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc include/linux/rcupdate.h
index 9fbc54a,585efef..0000000
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@@ -529,74 -517,12 +529,82 @@@ extern void call_rcu(struct rcu_head *h
  extern void call_rcu_bh(struct rcu_head *head,
  			void (*func)(struct rcu_head *head));
  
 +/*
 + * debug_rcu_head_queue()/debug_rcu_head_unqueue() are used internally
 + * by call_rcu() and rcu callback execution, and are therefore not part of the
 + * RCU API. Leaving in rcupdate.h because they are used by all RCU flavors.
 + */
 +
 +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
 +# define STATE_RCU_HEAD_READY	0
 +# define STATE_RCU_HEAD_QUEUED	1
 +
 +extern struct debug_obj_descr rcuhead_debug_descr;
 +
 +static inline void debug_rcu_head_queue(struct rcu_head *head)
 +{
 +	debug_object_activate(head, &rcuhead_debug_descr);
 +	debug_object_active_state(head, &rcuhead_debug_descr,
 +				  STATE_RCU_HEAD_READY,
 +				  STATE_RCU_HEAD_QUEUED);
 +}
 +
 +static inline void debug_rcu_head_unqueue(struct rcu_head *head)
 +{
 +	debug_object_active_state(head, &rcuhead_debug_descr,
 +				  STATE_RCU_HEAD_QUEUED,
 +				  STATE_RCU_HEAD_READY);
 +	debug_object_deactivate(head, &rcuhead_debug_descr);
 +}
 +#else	/* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 +static inline void debug_rcu_head_queue(struct rcu_head *head)
 +{
 +}
 +
 +static inline void debug_rcu_head_unqueue(struct rcu_head *head)
 +{
 +}
 +#endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 +
 +#ifndef CONFIG_PROVE_RCU
 +#define __do_rcu_dereference_check(c) do { } while (0)
 +#endif /* #ifdef CONFIG_PROVE_RCU */
 +
 +#define __rcu_dereference_index_check(p, c) \
 +	({ \
 +		typeof(p) _________p1 = ACCESS_ONCE(p); \
 +		__do_rcu_dereference_check(c); \
 +		smp_read_barrier_depends(); \
 +		(_________p1); \
 +	})
 +
 +/**
 + * rcu_dereference_index_check() - rcu_dereference for indices with debug checking
 + * @p: The pointer to read, prior to dereferencing
 + * @c: The conditions under which the dereference will take place
 + *
 + * Similar to rcu_dereference_check(), but omits the sparse checking.
 + * This allows rcu_dereference_index_check() to be used on integers,
 + * which can then be used as array indices.  Attempting to use
 + * rcu_dereference_check() on an integer will give compiler warnings
 + * because the sparse address-space mechanism relies on dereferencing
 + * the RCU-protected pointer.  Dereferencing integers is not something
 + * that even gcc will put up with.
 + *
 + * Note that this function does not implicitly check for RCU read-side
 + * critical sections.  If this function gains lots of uses, it might
 + * make sense to provide versions for each flavor of RCU, but it does
 + * not make sense as of early 2010.
 + */
 +#define rcu_dereference_index_check(p, c) \
 +	__rcu_dereference_index_check((p), (c))
 +
+ #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
+ extern void rcu_cpu_stall_reset(void);
+ #else /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
+ static void rcu_cpu_stall_reset()
+ {
+ }
+ #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
+ 
  #endif /* __LINUX_RCUPDATE_H */

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

* Re: linux-next: manual merge of the kgdb tree with Linus' tree
  2010-08-07  4:05 linux-next: manual merge of the kgdb tree with Linus' tree Stephen Rothwell
@ 2010-08-07 21:17 ` Paul E. McKenney
  2010-08-09  5:13   ` Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2010-08-07 21:17 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Jason Wessel, linux-next, linux-kernel, Mathieu Desnoyers

On Sat, Aug 07, 2010 at 02:05:42PM +1000, Stephen Rothwell wrote:
> Hi Jason,
> 
> Today's linux-next merge of the kgdb tree got a conflict in
> include/linux/rcupdate.h between commits
> 551d55a944b143ef26fbd482d1c463199d6f65cf ("tree/tiny rcu: Add debug RCU
> head objects") and f5155b33277c9678041a27869165619bb34f722f ("rcu: add an
> rcu_dereference_index_check()") from Linus' tree and commit
> 9e213357d0aeaeb81e213cfd3b9415db5fccc1b5 ("rcu,debug_core: allow the
> kernel debugger to reset the rcu stall timer") from the kgdb tree.

Hello, Jason,

Just trying to make sure I understand this...

This cannot be a "stop the machine" debugger, because otherwise the
jiffies counter would stop and you would not get RCU CPU stall warnings.

It might be a "stop the machine" debugger, but where the jiffies counter
catches up quickly as soon as the machine restarts.  In this case,
your patch would be a reasonable approach, but RCU CPU stall warnings
are going to be the least of your problems.  Actually, I have only seen
one piece of your patch.  Could you please send me the rest of it?

If you are permitting some tasks to run while others are halted,
then the RCU CPU stall is simply a symptom of an underlying problem,
namely that if you halt a task in an RCU read-side critical section
for long enough, you will OOM the system.

							Thanx, Paul

> Just overlapping additions.  I fixed it up (see below) and can carry the
> fix for a while.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc include/linux/rcupdate.h
> index 9fbc54a,585efef..0000000
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@@ -529,74 -517,12 +529,82 @@@ extern void call_rcu(struct rcu_head *h
>   extern void call_rcu_bh(struct rcu_head *head,
>   			void (*func)(struct rcu_head *head));
>   
>  +/*
>  + * debug_rcu_head_queue()/debug_rcu_head_unqueue() are used internally
>  + * by call_rcu() and rcu callback execution, and are therefore not part of the
>  + * RCU API. Leaving in rcupdate.h because they are used by all RCU flavors.
>  + */
>  +
>  +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
>  +# define STATE_RCU_HEAD_READY	0
>  +# define STATE_RCU_HEAD_QUEUED	1
>  +
>  +extern struct debug_obj_descr rcuhead_debug_descr;
>  +
>  +static inline void debug_rcu_head_queue(struct rcu_head *head)
>  +{
>  +	debug_object_activate(head, &rcuhead_debug_descr);
>  +	debug_object_active_state(head, &rcuhead_debug_descr,
>  +				  STATE_RCU_HEAD_READY,
>  +				  STATE_RCU_HEAD_QUEUED);
>  +}
>  +
>  +static inline void debug_rcu_head_unqueue(struct rcu_head *head)
>  +{
>  +	debug_object_active_state(head, &rcuhead_debug_descr,
>  +				  STATE_RCU_HEAD_QUEUED,
>  +				  STATE_RCU_HEAD_READY);
>  +	debug_object_deactivate(head, &rcuhead_debug_descr);
>  +}
>  +#else	/* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>  +static inline void debug_rcu_head_queue(struct rcu_head *head)
>  +{
>  +}
>  +
>  +static inline void debug_rcu_head_unqueue(struct rcu_head *head)
>  +{
>  +}
>  +#endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>  +
>  +#ifndef CONFIG_PROVE_RCU
>  +#define __do_rcu_dereference_check(c) do { } while (0)
>  +#endif /* #ifdef CONFIG_PROVE_RCU */
>  +
>  +#define __rcu_dereference_index_check(p, c) \
>  +	({ \
>  +		typeof(p) _________p1 = ACCESS_ONCE(p); \
>  +		__do_rcu_dereference_check(c); \
>  +		smp_read_barrier_depends(); \
>  +		(_________p1); \
>  +	})
>  +
>  +/**
>  + * rcu_dereference_index_check() - rcu_dereference for indices with debug checking
>  + * @p: The pointer to read, prior to dereferencing
>  + * @c: The conditions under which the dereference will take place
>  + *
>  + * Similar to rcu_dereference_check(), but omits the sparse checking.
>  + * This allows rcu_dereference_index_check() to be used on integers,
>  + * which can then be used as array indices.  Attempting to use
>  + * rcu_dereference_check() on an integer will give compiler warnings
>  + * because the sparse address-space mechanism relies on dereferencing
>  + * the RCU-protected pointer.  Dereferencing integers is not something
>  + * that even gcc will put up with.
>  + *
>  + * Note that this function does not implicitly check for RCU read-side
>  + * critical sections.  If this function gains lots of uses, it might
>  + * make sense to provide versions for each flavor of RCU, but it does
>  + * not make sense as of early 2010.
>  + */
>  +#define rcu_dereference_index_check(p, c) \
>  +	__rcu_dereference_index_check((p), (c))
>  +
> + #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
> + extern void rcu_cpu_stall_reset(void);
> + #else /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
> + static void rcu_cpu_stall_reset()
> + {
> + }
> + #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
> + 
>   #endif /* __LINUX_RCUPDATE_H */

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

* Re: linux-next: manual merge of the kgdb tree with Linus' tree
  2010-08-07 21:17 ` Paul E. McKenney
@ 2010-08-09  5:13   ` Jason Wessel
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Wessel @ 2010-08-09  5:13 UTC (permalink / raw)
  To: paulmck; +Cc: Stephen Rothwell, linux-next, linux-kernel, Mathieu Desnoyers

On 08/07/2010 04:17 PM, Paul E. McKenney wrote:
> On Sat, Aug 07, 2010 at 02:05:42PM +1000, Stephen Rothwell wrote:
>   
>> Hi Jason,
>>
>> Today's linux-next merge of the kgdb tree got a conflict in
>> include/linux/rcupdate.h between commits
>> 551d55a944b143ef26fbd482d1c463199d6f65cf ("tree/tiny rcu: Add debug RCU
>> head objects") and f5155b33277c9678041a27869165619bb34f722f ("rcu: add an
>> rcu_dereference_index_check()") from Linus' tree and commit
>> 9e213357d0aeaeb81e213cfd3b9415db5fccc1b5 ("rcu,debug_core: allow the
>> kernel debugger to reset the rcu stall timer") from the kgdb tree.
>>     
>
> Hello, Jason,
>
> Just trying to make sure I understand this...
>
> This cannot be a "stop the machine" debugger, because otherwise the
> jiffies counter would stop and you would not get RCU CPU stall warnings.
>
> It might be a "stop the machine" debugger, but where the jiffies counter
> catches up quickly as soon as the machine restarts.  In this case,
> your patch would be a reasonable approach, but RCU CPU stall warnings
> are going to be the least of your problems. 

You should have the patches now in as I posted them to LKML as an RFC.  
If there are other problems in this area I am interested in
understanding what further issues exist that still have yet to be dealt
with.

The general idea is that the kernel can take an exception and execute
for a short period of time with all the processors spinning in a wait
loop and then resume kernel execution.  As you might guess the debugger
is a "multipurpose" tool and there are quite a few circumstances where
the a trip into the debugger is really a one way trip to a reboot when
you are done inspecting.

>  Actually, I have only seen
> one piece of your patch.  Could you please send me the rest of it?
>
> If you are permitting some tasks to run while others are halted,
> then the RCU CPU stall is simply a symptom of an underlying problem,
> namely that if you halt a task in an RCU read-side critical section
> for long enough, you will OOM the system.
>
>   

We are definitely not "partially running".  Picking an choosing threads
to run without a complete integration with the scheduler and all other
related systems like RCU would be a _really_ bad idea.  :-)

Jason.

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

* Re: linux-next: manual merge of the kgdb tree with Linus' tree
  2010-07-23  3:38 ` Jason Wessel
@ 2010-07-23  7:08   ` Stephen Rothwell
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Rothwell @ 2010-07-23  7:08 UTC (permalink / raw)
  To: Jason Wessel; +Cc: linux-next, linux-kernel

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

Hi Jason,

On Thu, 22 Jul 2010 22:38:24 -0500 Jason Wessel <jason.wessel@windriver.com> wrote:
>
> This already fixed.  The problem was caused by the for_linus pull branch
> being out of sync with kgdb-next.

Yeah, I guessed :-)

Thanks

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

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

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

* Re: linux-next: manual merge of the kgdb tree with Linus' tree
  2010-07-23  2:26 Stephen Rothwell
@ 2010-07-23  3:38 ` Jason Wessel
  2010-07-23  7:08   ` Stephen Rothwell
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2010-07-23  3:38 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linux-next, linux-kernel

On 07/22/2010 09:26 PM, Stephen Rothwell wrote:
> Hi Jason,
>
> Today's linux-next merge of the kgdb tree got a conflict in
> kernel/debug/kdb/kdb_main.c between commit
> edd63cb6b91024332d6983fc51058ac1ef0c081e ("sysrq,kdb: Use __handle_sysrq
> () for kdb's sysrq function") from Linus' tree and commit
> 93c47fada46e034941106fb34b76a00a87108fc5 ("kdb: Restore previous sysrq
> state") from the kgdb tree.
>
> These are two fixes for the same problem.  I effectively reverted the one
> from the kgdb tree.
>   

This already fixed.  The problem was caused by the for_linus pull branch
being out of sync with kgdb-next.

Thanks,
Jason.

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

* linux-next: manual merge of the kgdb tree with Linus' tree
@ 2010-07-23  2:26 Stephen Rothwell
  2010-07-23  3:38 ` Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2010-07-23  2:26 UTC (permalink / raw)
  To: Jason Wessel; +Cc: linux-next, linux-kernel

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

Hi Jason,

Today's linux-next merge of the kgdb tree got a conflict in
kernel/debug/kdb/kdb_main.c between commit
edd63cb6b91024332d6983fc51058ac1ef0c081e ("sysrq,kdb: Use __handle_sysrq
() for kdb's sysrq function") from Linus' tree and commit
93c47fada46e034941106fb34b76a00a87108fc5 ("kdb: Restore previous sysrq
state") from the kgdb tree.

These are two fixes for the same problem.  I effectively reverted the one
from the kgdb tree.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

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

end of thread, other threads:[~2010-08-09  5:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-07  4:05 linux-next: manual merge of the kgdb tree with Linus' tree Stephen Rothwell
2010-08-07 21:17 ` Paul E. McKenney
2010-08-09  5:13   ` Jason Wessel
  -- strict thread matches above, loose matches on Subject: below --
2010-07-23  2:26 Stephen Rothwell
2010-07-23  3:38 ` Jason Wessel
2010-07-23  7:08   ` Stephen Rothwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).