Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 08:22:50PM -0800, Josh Triplett wrote: > On Mon, Jan 07, 2013 at 02:19:15PM -0800, Paul E. McKenney wrote: > > On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: > > > On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: > > > > On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: > > > > > This patch seems reasonable to me, but the repeated use of #if > > > > > defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat > > > > > annoying, and fragile if you ever decide to change the conditions. > > > > > How > > > > > about defining an appropriate symbol in Kconfig for stall warnings, > > > > > and > > > > > using that? > > > > > > > > But I only just removed the config option for SMP RCU stall warnings. > > > > ;-) > > > > > > > > But I must agree that "defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE)" > > > > is a bit obscure. The rationale is that RCU stall warnings are > > > > unconditionally enabled in SMP kernels, but don't want to be in > > > > TINY_RCU kernels due to size constraints. I therefore put it under > > > > CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style > > > > options. Would adding a comment to this effect help? > > > > > > I understand the rationale; I just think it would become clearer if you > > > added an internal-only Kconfig symbol selected in both cases and change > > > the conditionals to use that. > > > > My concern was that this would confuse people into thinking that the > > code under those #ifdefs was all the stall-warning code that there was. > > > > I suppose this could be forestalled with a suitably clever name... > > CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? > > How about CONFIG_RCU_STALL_COMMON, with associated help text saying > "include the stall-detection code common to both rcutree and rcutiny"? Sold!!! Especially given that I am creating the commit to allow TREE_PREEMPT_RCU to be used on UP systems with an eye towards getting rid of TINY_PREEMPT_RCU. ;-) Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 08:22:50PM -0800, Josh Triplett wrote: On Mon, Jan 07, 2013 at 02:19:15PM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? I understand the rationale; I just think it would become clearer if you added an internal-only Kconfig symbol selected in both cases and change the conditionals to use that. My concern was that this would confuse people into thinking that the code under those #ifdefs was all the stall-warning code that there was. I suppose this could be forestalled with a suitably clever name... CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? How about CONFIG_RCU_STALL_COMMON, with associated help text saying include the stall-detection code common to both rcutree and rcutiny? Sold!!! Especially given that I am creating the commit to allow TREE_PREEMPT_RCU to be used on UP systems with an eye towards getting rid of TINY_PREEMPT_RCU. ;-) Thanx, Paul -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 02:19:15PM -0800, Paul E. McKenney wrote: > On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: > > On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: > > > On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: > > > > This patch seems reasonable to me, but the repeated use of #if > > > > defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat > > > > annoying, and fragile if you ever decide to change the conditions. How > > > > about defining an appropriate symbol in Kconfig for stall warnings, and > > > > using that? > > > > > > But I only just removed the config option for SMP RCU stall warnings. ;-) > > > > > > But I must agree that "defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE)" > > > is a bit obscure. The rationale is that RCU stall warnings are > > > unconditionally enabled in SMP kernels, but don't want to be in > > > TINY_RCU kernels due to size constraints. I therefore put it under > > > CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style > > > options. Would adding a comment to this effect help? > > > > I understand the rationale; I just think it would become clearer if you > > added an internal-only Kconfig symbol selected in both cases and change > > the conditionals to use that. > > My concern was that this would confuse people into thinking that the > code under those #ifdefs was all the stall-warning code that there was. > > I suppose this could be forestalled with a suitably clever name... > CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? How about CONFIG_RCU_STALL_COMMON, with associated help text saying "include the stall-detection code common to both rcutree and rcutiny"? - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: > On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: > > On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: > > > On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: > > > > rcu: Provide RCU CPU stall warnings for tiny RCU > > > > > > > > Tiny RCU has historically omitted RCU CPU stall warnings in order to > > > > reduce memory requirements, however, lack of these warnings caused > > > > Thomas > > > > Gleixner some debugging pain recently. Therefore, this commit adds RCU > > > > CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory > > > > footprint small, while still enabling CPU stall warnings in kernels > > > > built to enable them. > > > > > > > > This is still a bit on the high-risk side, so running this will likely > > > > be a debugging exercise. > > > > > > > > Reported-by: Thomas Gleixner > > > > Signed-off-by: Paul E. McKenney > > > > Signed-off-by: Paul E. McKenney > > > > > > Did you generate this patch with something other than git? The > > > formatting seems a bit off: it doesn't have a diffstat or the usual > > > "---" line between the commit message and the patch. > > > > Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 > > series of patches. ;-) > > Just don't pass --cover-letter to git format-patch and you won't get the > 0/1. Ah, good point! Thank you! > > > This patch seems reasonable to me, but the repeated use of #if > > > defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat > > > annoying, and fragile if you ever decide to change the conditions. How > > > about defining an appropriate symbol in Kconfig for stall warnings, and > > > using that? > > > > But I only just removed the config option for SMP RCU stall warnings. ;-) > > > > But I must agree that "defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE)" > > is a bit obscure. The rationale is that RCU stall warnings are > > unconditionally enabled in SMP kernels, but don't want to be in > > TINY_RCU kernels due to size constraints. I therefore put it under > > CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style > > options. Would adding a comment to this effect help? > > I understand the rationale; I just think it would become clearer if you > added an internal-only Kconfig symbol selected in both cases and change > the conditionals to use that. My concern was that this would confuse people into thinking that the code under those #ifdefs was all the stall-warning code that there was. I suppose this could be forestalled with a suitably clever name... CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: > On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: > > On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: > > > rcu: Provide RCU CPU stall warnings for tiny RCU > > > > > > Tiny RCU has historically omitted RCU CPU stall warnings in order to > > > reduce memory requirements, however, lack of these warnings caused Thomas > > > Gleixner some debugging pain recently. Therefore, this commit adds RCU > > > CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory > > > footprint small, while still enabling CPU stall warnings in kernels > > > built to enable them. > > > > > > This is still a bit on the high-risk side, so running this will likely > > > be a debugging exercise. > > > > > > Reported-by: Thomas Gleixner > > > Signed-off-by: Paul E. McKenney > > > Signed-off-by: Paul E. McKenney > > > > Did you generate this patch with something other than git? The > > formatting seems a bit off: it doesn't have a diffstat or the usual > > "---" line between the commit message and the patch. > > Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 > series of patches. ;-) Just don't pass --cover-letter to git format-patch and you won't get the 0/1. > > This patch seems reasonable to me, but the repeated use of #if > > defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat > > annoying, and fragile if you ever decide to change the conditions. How > > about defining an appropriate symbol in Kconfig for stall warnings, and > > using that? > > But I only just removed the config option for SMP RCU stall warnings. ;-) > > But I must agree that "defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE)" > is a bit obscure. The rationale is that RCU stall warnings are > unconditionally enabled in SMP kernels, but don't want to be in > TINY_RCU kernels due to size constraints. I therefore put it under > CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style > options. Would adding a comment to this effect help? I understand the rationale; I just think it would become clearer if you added an internal-only Kconfig symbol selected in both cases and change the conditionals to use that. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: > On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: > > rcu: Provide RCU CPU stall warnings for tiny RCU > > > > Tiny RCU has historically omitted RCU CPU stall warnings in order to > > reduce memory requirements, however, lack of these warnings caused Thomas > > Gleixner some debugging pain recently. Therefore, this commit adds RCU > > CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory > > footprint small, while still enabling CPU stall warnings in kernels > > built to enable them. > > > > This is still a bit on the high-risk side, so running this will likely > > be a debugging exercise. > > > > Reported-by: Thomas Gleixner > > Signed-off-by: Paul E. McKenney > > Signed-off-by: Paul E. McKenney > > Did you generate this patch with something other than git? The > formatting seems a bit off: it doesn't have a diffstat or the usual > "---" line between the commit message and the patch. Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 series of patches. ;-) > This patch seems reasonable to me, but the repeated use of #if > defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat > annoying, and fragile if you ever decide to change the conditions. How > about defining an appropriate symbol in Kconfig for stall warnings, and > using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that "defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE)" is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: > rcu: Provide RCU CPU stall warnings for tiny RCU > > Tiny RCU has historically omitted RCU CPU stall warnings in order to > reduce memory requirements, however, lack of these warnings caused Thomas > Gleixner some debugging pain recently. Therefore, this commit adds RCU > CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory > footprint small, while still enabling CPU stall warnings in kernels > built to enable them. > > This is still a bit on the high-risk side, so running this will likely > be a debugging exercise. > > Reported-by: Thomas Gleixner > Signed-off-by: Paul E. McKenney > Signed-off-by: Paul E. McKenney Did you generate this patch with something other than git? The formatting seems a bit off: it doesn't have a diffstat or the usual "---" line between the commit message and the patch. This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner t...@linutronix.de Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com Did you generate this patch with something other than git? The formatting seems a bit off: it doesn't have a diffstat or the usual --- line between the commit message and the patch. This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? - Josh Triplett -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner t...@linutronix.de Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com Did you generate this patch with something other than git? The formatting seems a bit off: it doesn't have a diffstat or the usual --- line between the commit message and the patch. Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 series of patches. ;-) This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? Thanx, Paul -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner t...@linutronix.de Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com Did you generate this patch with something other than git? The formatting seems a bit off: it doesn't have a diffstat or the usual --- line between the commit message and the patch. Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 series of patches. ;-) Just don't pass --cover-letter to git format-patch and you won't get the 0/1. This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? I understand the rationale; I just think it would become clearer if you added an internal-only Kconfig symbol selected in both cases and change the conditionals to use that. - Josh Triplett -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: On Sat, Jan 05, 2013 at 09:50:59AM -0800, Paul E. McKenney wrote: rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner t...@linutronix.de Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com Did you generate this patch with something other than git? The formatting seems a bit off: it doesn't have a diffstat or the usual --- line between the commit message and the patch. Indeed I did -- couldn't see the point of sending a 0/1 and 1/1 series of patches. ;-) Just don't pass --cover-letter to git format-patch and you won't get the 0/1. Ah, good point! Thank you! This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? I understand the rationale; I just think it would become clearer if you added an internal-only Kconfig symbol selected in both cases and change the conditionals to use that. My concern was that this would confuse people into thinking that the code under those #ifdefs was all the stall-warning code that there was. I suppose this could be forestalled with a suitably clever name... CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? Thanx, Paul -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
On Mon, Jan 07, 2013 at 02:19:15PM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 09:56:06AM -0800, Josh Triplett wrote: On Mon, Jan 07, 2013 at 08:57:48AM -0800, Paul E. McKenney wrote: On Mon, Jan 07, 2013 at 07:58:10AM -0800, Josh Triplett wrote: This patch seems reasonable to me, but the repeated use of #if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) seems somewhat annoying, and fragile if you ever decide to change the conditions. How about defining an appropriate symbol in Kconfig for stall warnings, and using that? But I only just removed the config option for SMP RCU stall warnings. ;-) But I must agree that defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) is a bit obscure. The rationale is that RCU stall warnings are unconditionally enabled in SMP kernels, but don't want to be in TINY_RCU kernels due to size constraints. I therefore put it under CONFIG_RCU_TRACE, which also contains other TINY_RCU debugging-style options. Would adding a comment to this effect help? I understand the rationale; I just think it would become clearer if you added an internal-only Kconfig symbol selected in both cases and change the conditionals to use that. My concern was that this would confuse people into thinking that the code under those #ifdefs was all the stall-warning code that there was. I suppose this could be forestalled with a suitably clever name... CONFIG_RCU_CPU_STALL_TINY_TOO? Better names? How about CONFIG_RCU_STALL_COMMON, with associated help text saying include the stall-detection code common to both rcutree and rcutiny? - Josh Triplett -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner Signed-off-by: Paul E. McKenney Signed-off-by: Paul E. McKenney diff --git a/kernel/rcu.h b/kernel/rcu.h index 20dfba5..7ff057d 100644 --- a/kernel/rcu.h +++ b/kernel/rcu.h @@ -111,4 +111,11 @@ static inline bool __rcu_reclaim(char *rn, struct rcu_head *head) extern int rcu_expedited; +#if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) + +extern int rcu_cpu_stall_suppress; +int rcu_jiffies_till_stall_check(void); + +#endif /* defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) */ + #endif /* __LINUX_RCU_H */ diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c index a2cf761..06cec61 100644 --- a/kernel/rcupdate.c +++ b/kernel/rcupdate.c @@ -412,3 +412,54 @@ EXPORT_SYMBOL_GPL(do_trace_rcu_torture_read); #else #define do_trace_rcu_torture_read(rcutorturename, rhp) do { } while (0) #endif + +#if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) + +#ifdef CONFIG_PROVE_RCU +#define RCU_STALL_DELAY_DELTA (5 * HZ) +#else +#define RCU_STALL_DELAY_DELTA 0 +#endif + +int rcu_cpu_stall_suppress __read_mostly; /* 1 = suppress stall warnings. */ +int rcu_cpu_stall_timeout __read_mostly = CONFIG_RCU_CPU_STALL_TIMEOUT; + +module_param(rcu_cpu_stall_suppress, int, 0644); +module_param(rcu_cpu_stall_timeout, int, 0644); + +int rcu_jiffies_till_stall_check(void) +{ + int till_stall_check = ACCESS_ONCE(rcu_cpu_stall_timeout); + + /* +* Limit check must be consistent with the Kconfig limits +* for CONFIG_RCU_CPU_STALL_TIMEOUT. +*/ + if (till_stall_check < 3) { + ACCESS_ONCE(rcu_cpu_stall_timeout) = 3; + till_stall_check = 3; + } else if (till_stall_check > 300) { + ACCESS_ONCE(rcu_cpu_stall_timeout) = 300; + till_stall_check = 300; + } + return till_stall_check * HZ + RCU_STALL_DELAY_DELTA; +} + +static int rcu_panic(struct notifier_block *this, unsigned long ev, void *ptr) +{ + rcu_cpu_stall_suppress = 1; + return NOTIFY_DONE; +} + +static struct notifier_block rcu_panic_block = { + .notifier_call = rcu_panic, +}; + +static int __init check_cpu_stall_init(void) +{ + atomic_notifier_chain_register(_notifier_list, _panic_block); + return 0; +} +early_initcall(check_cpu_stall_init); + +#endif /* defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) */ diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c index e7dce58..b899df3 100644 --- a/kernel/rcutiny.c +++ b/kernel/rcutiny.c @@ -51,10 +51,10 @@ static void __call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu), struct rcu_ctrlblk *rcp); -#include "rcutiny_plugin.h" - static long long rcu_dynticks_nesting = DYNTICK_TASK_EXIT_IDLE; +#include "rcutiny_plugin.h" + /* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcutree.c. */ static void rcu_idle_enter_common(long long newval) { @@ -205,6 +205,7 @@ int rcu_is_cpu_rrupt_from_idle(void) */ static int rcu_qsctr_help(struct rcu_ctrlblk *rcp) { + reset_cpu_stall_ticks(rcp); if (rcp->rcucblist != NULL && rcp->donetail != rcp->curtail) { rcp->donetail = rcp->curtail; @@ -251,6 +252,7 @@ void rcu_bh_qs(int cpu) */ void rcu_check_callbacks(int cpu, int user) { + check_cpu_stalls(); if (user || rcu_is_cpu_rrupt_from_idle()) rcu_sched_qs(cpu); else if (!in_softirq()) diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h index f85016a..8a23300 100644 --- a/kernel/rcutiny_plugin.h +++ b/kernel/rcutiny_plugin.h @@ -33,6 +33,9 @@ struct rcu_ctrlblk { struct rcu_head **donetail; /* ->next pointer of last "done" CB. */ struct rcu_head **curtail; /* ->next pointer of last CB. */ RCU_TRACE(long qlen); /* Number of pending CBs. */ + RCU_TRACE(unsigned long gp_start); /* Start time for stalls. */ + RCU_TRACE(unsigned long ticks_this_gp); /* Statistic for stalls. */ + RCU_TRACE(unsigned long jiffies_stall); /* Jiffies at next stall. */ RCU_TRACE(char *name); /* Name of RCU type. */ }; @@ -54,6 +57,51 @@ int rcu_scheduler_active __read_mostly; EXPORT_SYMBOL_GPL(rcu_scheduler_active); #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ +#ifdef CONFIG_RCU_TRACE + +static void check_cpu_stall(struct rcu_ctrlblk *rcp) +{ + unsigned long j; + unsigned long js; + + if
[PATCH tip/core/rcu 1/1] Tiny RCU changes for 3.9
rcu: Provide RCU CPU stall warnings for tiny RCU Tiny RCU has historically omitted RCU CPU stall warnings in order to reduce memory requirements, however, lack of these warnings caused Thomas Gleixner some debugging pain recently. Therefore, this commit adds RCU CPU stall warnings to tiny RCU if RCU_TRACE=y. This keeps the memory footprint small, while still enabling CPU stall warnings in kernels built to enable them. This is still a bit on the high-risk side, so running this will likely be a debugging exercise. Reported-by: Thomas Gleixner t...@linutronix.de Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com diff --git a/kernel/rcu.h b/kernel/rcu.h index 20dfba5..7ff057d 100644 --- a/kernel/rcu.h +++ b/kernel/rcu.h @@ -111,4 +111,11 @@ static inline bool __rcu_reclaim(char *rn, struct rcu_head *head) extern int rcu_expedited; +#if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) + +extern int rcu_cpu_stall_suppress; +int rcu_jiffies_till_stall_check(void); + +#endif /* defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) */ + #endif /* __LINUX_RCU_H */ diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c index a2cf761..06cec61 100644 --- a/kernel/rcupdate.c +++ b/kernel/rcupdate.c @@ -412,3 +412,54 @@ EXPORT_SYMBOL_GPL(do_trace_rcu_torture_read); #else #define do_trace_rcu_torture_read(rcutorturename, rhp) do { } while (0) #endif + +#if defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) + +#ifdef CONFIG_PROVE_RCU +#define RCU_STALL_DELAY_DELTA (5 * HZ) +#else +#define RCU_STALL_DELAY_DELTA 0 +#endif + +int rcu_cpu_stall_suppress __read_mostly; /* 1 = suppress stall warnings. */ +int rcu_cpu_stall_timeout __read_mostly = CONFIG_RCU_CPU_STALL_TIMEOUT; + +module_param(rcu_cpu_stall_suppress, int, 0644); +module_param(rcu_cpu_stall_timeout, int, 0644); + +int rcu_jiffies_till_stall_check(void) +{ + int till_stall_check = ACCESS_ONCE(rcu_cpu_stall_timeout); + + /* +* Limit check must be consistent with the Kconfig limits +* for CONFIG_RCU_CPU_STALL_TIMEOUT. +*/ + if (till_stall_check 3) { + ACCESS_ONCE(rcu_cpu_stall_timeout) = 3; + till_stall_check = 3; + } else if (till_stall_check 300) { + ACCESS_ONCE(rcu_cpu_stall_timeout) = 300; + till_stall_check = 300; + } + return till_stall_check * HZ + RCU_STALL_DELAY_DELTA; +} + +static int rcu_panic(struct notifier_block *this, unsigned long ev, void *ptr) +{ + rcu_cpu_stall_suppress = 1; + return NOTIFY_DONE; +} + +static struct notifier_block rcu_panic_block = { + .notifier_call = rcu_panic, +}; + +static int __init check_cpu_stall_init(void) +{ + atomic_notifier_chain_register(panic_notifier_list, rcu_panic_block); + return 0; +} +early_initcall(check_cpu_stall_init); + +#endif /* defined(CONFIG_SMP) || defined(CONFIG_RCU_TRACE) */ diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c index e7dce58..b899df3 100644 --- a/kernel/rcutiny.c +++ b/kernel/rcutiny.c @@ -51,10 +51,10 @@ static void __call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu), struct rcu_ctrlblk *rcp); -#include rcutiny_plugin.h - static long long rcu_dynticks_nesting = DYNTICK_TASK_EXIT_IDLE; +#include rcutiny_plugin.h + /* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcutree.c. */ static void rcu_idle_enter_common(long long newval) { @@ -205,6 +205,7 @@ int rcu_is_cpu_rrupt_from_idle(void) */ static int rcu_qsctr_help(struct rcu_ctrlblk *rcp) { + reset_cpu_stall_ticks(rcp); if (rcp-rcucblist != NULL rcp-donetail != rcp-curtail) { rcp-donetail = rcp-curtail; @@ -251,6 +252,7 @@ void rcu_bh_qs(int cpu) */ void rcu_check_callbacks(int cpu, int user) { + check_cpu_stalls(); if (user || rcu_is_cpu_rrupt_from_idle()) rcu_sched_qs(cpu); else if (!in_softirq()) diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h index f85016a..8a23300 100644 --- a/kernel/rcutiny_plugin.h +++ b/kernel/rcutiny_plugin.h @@ -33,6 +33,9 @@ struct rcu_ctrlblk { struct rcu_head **donetail; /* -next pointer of last done CB. */ struct rcu_head **curtail; /* -next pointer of last CB. */ RCU_TRACE(long qlen); /* Number of pending CBs. */ + RCU_TRACE(unsigned long gp_start); /* Start time for stalls. */ + RCU_TRACE(unsigned long ticks_this_gp); /* Statistic for stalls. */ + RCU_TRACE(unsigned long jiffies_stall); /* Jiffies at next stall. */ RCU_TRACE(char *name); /* Name of RCU type. */ }; @@ -54,6 +57,51 @@ int rcu_scheduler_active __read_mostly; EXPORT_SYMBOL_GPL(rcu_scheduler_active); #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ +#ifdef CONFIG_RCU_TRACE + +static void check_cpu_stall(struct rcu_ctrlblk *rcp) +{ + unsigned