[PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Li Zefan]

CONFIG_DETECT_SOFTLOCKUP has been removed, so switch the
default value to LOCKUP_DETECTOR.

Also fix the help text of BOOT_PRINTK_DELAY.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 lib/Kconfig.debug |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 596106b..2fb7fa3 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -203,7 +203,7 @@ config BOOTPARAM_SOFTLOCKUP_PANIC_VALUE
 config DETECT_HUNG_TASK
 	bool "Detect Hung Tasks"
 	depends on DEBUG_KERNEL
-	default DETECT_SOFTLOCKUP
+	default LOCKUP_DETECTOR
 	help
 	  Say Y here to enable the kernel to detect "hung tasks",
 	  which are bugs that cause the task to be stuck in
@@ -767,7 +767,7 @@ config BOOT_PRINTK_DELAY
 	  system, and then set "lpj=M" before setting "boot_delay=N".
 	  NOTE:  Using this option may adversely affect SMP systems.
 	  I.e., processors other than the first one may not boot up.
-	  BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
+	  BOOT_PRINTK_DELAY also may cause LOCKUP_DETECTOR to detect
 	  what it believes to be lockup conditions.
 
 config RCU_TORTURE_TEST
-- 
1.6.3

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Frederic Weisbecker]

On Wed, Aug 04, 2010 at 09:53:03AM +0800, Li Zefan wrote:
> CONFIG_DETECT_SOFTLOCKUP has been removed, so switch the
> default value to LOCKUP_DETECTOR.
> 
> Also fix the help text of BOOT_PRINTK_DELAY.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---


Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Thanks.

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Ingo Molnar]

(Linus Cc:-ed)

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Wed, Aug 04, 2010 at 09:53:03AM +0800, Li Zefan wrote:
> > CONFIG_DETECT_SOFTLOCKUP has been removed, so switch the
> > default value to LOCKUP_DETECTOR.
> > 
> > Also fix the help text of BOOT_PRINTK_DELAY.
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > ---
> 
> 
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> Thanks.

The thing is, CONFIG_DETECT_SOFTLOCKUP was default-y before, so many people 
had it enabled [and had it forced-enabled if DEBUG_KERNEL was off], even if 
they didnt really want or need it.

So i turned off the new generic watchdog code's default intentionally - as it 
clearly does not cure cancer ;-)

I think distros will enable it, and most testers will as well. Those who dont 
enable it and run into a lockup have an easy option to enable.

Maybe a better change would be to make it more generally available - right now 
it's:

 config LOCKUP_DETECTOR
         bool "Detect Hard and Soft Lockups"
         depends on DEBUG_KERNEL && !S390

which means that it cannot be enabled when DEBUG_KERNEL is off.

So i think we should:

 - Remove the s390 hack and add an ARCH_HAS_LOCKUP_DETECTOR flag
 - Remove the DEBUG_KERNEL dependency

Hm?

Thanks,

	Ingo

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Frederic Weisbecker]

(More Cc)

On Sat, Aug 07, 2010 at 09:01:35AM +0200, Ingo Molnar wrote:
> 
> (Linus Cc:-ed)
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > On Wed, Aug 04, 2010 at 09:53:03AM +0800, Li Zefan wrote:
> > > CONFIG_DETECT_SOFTLOCKUP has been removed, so switch the
> > > default value to LOCKUP_DETECTOR.
> > > 
> > > Also fix the help text of BOOT_PRINTK_DELAY.
> > > 
> > > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > > ---
> > 
> > 
> > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> > 
> > Thanks.
> 
> The thing is, CONFIG_DETECT_SOFTLOCKUP was default-y before, so many people 
> had it enabled [and had it forced-enabled if DEBUG_KERNEL was off], even if 
> they didnt really want or need it.



Hmm. It was:

config DETECT_SOFTLOCKUP
	bool "Detect Soft Lockups"
	depends on DEBUG_KERNEL && !S390
	default y


It means it's default enabled only if DEBUG_KERNEL, right?
Then if you don't select CONFIG_DEBUG_KERNEL, it's fine as it won't
be selected.

But I agree with you. There is a bunch of config options for which
selection is a duty when you are a kernel developer:
PROVE_LOCKING, DETECT_HUNG_TASK, DEBUG_PREEMPT, PROVE_RCU, etc...
Because they all show (or prove we can have) bugs that one might miss
without these options. Softlockups are rarely part of them because even
without the lockup detector enabled, you'll observe something is wrong.



> So i turned off the new generic watchdog code's default intentionally - as it 
> clearly does not cure cancer ;-)


:-)


 
> I think distros will enable it, and most testers will as well. Those who dont 
> enable it and run into a lockup have an easy option to enable.



Why distros would want to enable it? The lockup detector introduces overhead.



> Maybe a better change would be to make it more generally available - right now 
> it's:
> 
>  config LOCKUP_DETECTOR
>          bool "Detect Hard and Soft Lockups"
>          depends on DEBUG_KERNEL && !S390
> 
> which means that it cannot be enabled when DEBUG_KERNEL is off.
> 
> So i think we should:
> 
>  - Remove the s390 hack and add an ARCH_HAS_LOCKUP_DETECTOR flag



If we do this, we'll need to add this config on every archs but s390.
We should better have ARCH_WANT_NO_LOCKUP_DETECTOR. I know that
"negative" meaning configs suck, but otherwise we would lose this
support on many archs.

Why s390 doesn't want the softlockup detector to begin with?



>  - Remove the DEBUG_KERNEL dependency


Yeah.

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> > The thing is, CONFIG_DETECT_SOFTLOCKUP was default-y before, so many 
> > people had it enabled [and had it forced-enabled if DEBUG_KERNEL was off], 
> > even if they didnt really want or need it.
> 
> Hmm. It was:
> 
> config DETECT_SOFTLOCKUP
> 	bool "Detect Soft Lockups"
> 	depends on DEBUG_KERNEL && !S390
> 	default y
> 
> It means it's default enabled only if DEBUG_KERNEL, right? Then if you don't 
> select CONFIG_DEBUG_KERNEL, it's fine as it won't be selected.

Indeed, you are right.

Anyway, i think the general point remains: i'm not sure we should 
default-enable this feature.

> But I agree with you. There is a bunch of config options for which selection 
> is a duty when you are a kernel developer: PROVE_LOCKING, DETECT_HUNG_TASK, 
> DEBUG_PREEMPT, PROVE_RCU, etc... Because they all show (or prove we can 
> have) bugs that one might miss without these options. Softlockups are rarely 
> part of them because even without the lockup detector enabled, you'll 
> observe something is wrong.

Note that it's now detecting all kinds of lockups: softlockups, hard lockups 
and even unkillable hung tasks.

	Ingo

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Frederic Weisbecker]

On Sun, Aug 08, 2010 at 11:23:11PM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > > The thing is, CONFIG_DETECT_SOFTLOCKUP was default-y before, so many 
> > > people had it enabled [and had it forced-enabled if DEBUG_KERNEL was off], 
> > > even if they didnt really want or need it.
> > 
> > Hmm. It was:
> > 
> > config DETECT_SOFTLOCKUP
> > 	bool "Detect Soft Lockups"
> > 	depends on DEBUG_KERNEL && !S390
> > 	default y
> > 
> > It means it's default enabled only if DEBUG_KERNEL, right? Then if you don't 
> > select CONFIG_DEBUG_KERNEL, it's fine as it won't be selected.
> 
> Indeed, you are right.
> 
> Anyway, i think the general point remains: i'm not sure we should 
> default-enable this feature.



Yeah, right.


 
> > But I agree with you. There is a bunch of config options for which selection 
> > is a duty when you are a kernel developer: PROVE_LOCKING, DETECT_HUNG_TASK, 
> > DEBUG_PREEMPT, PROVE_RCU, etc... Because they all show (or prove we can 
> > have) bugs that one might miss without these options. Softlockups are rarely 
> > part of them because even without the lockup detector enabled, you'll 
> > observe something is wrong.
> 
> Note that it's now detecting all kinds of lockups: softlockups, hard lockups 
> and even unkillable hung tasks.
> 
> 	Ingo



The unkillable hung task detector remains seperate. May be from the config point
of view it could be joined, but from an implementation point of view it has too
few to share with the lockup detector: it doesn't need a real time task, nor
a timer, etc...

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Heiko Carstens]

On Sun, Aug 08, 2010 at 09:58:42PM +0200, Frederic Weisbecker wrote:
> > Maybe a better change would be to make it more generally available - right now 
> > it's:
> > 
> >  config LOCKUP_DETECTOR
> >          bool "Detect Hard and Soft Lockups"
> >          depends on DEBUG_KERNEL && !S390
> > 
> > which means that it cannot be enabled when DEBUG_KERNEL is off.
> > 
> > So i think we should:
> > 
> >  - Remove the s390 hack and add an ARCH_HAS_LOCKUP_DETECTOR flag
> 
> 
> 
> If we do this, we'll need to add this config on every archs but s390.
> We should better have ARCH_WANT_NO_LOCKUP_DETECTOR. I know that
> "negative" meaning configs suck, but otherwise we would lose this
> support on many archs.
> 
> Why s390 doesn't want the softlockup detector to begin with?

If I remember correctly then we disabled that back then because we got
false positives. The reason for those were that the softlockup detector
did not take steal time into account.
E.g. if a guest cpu runs for 10 seconds, but the hypervisor would steal
9 seconds in order to run other guest cpus this specific cpu would still
think it ran for 10 seconds and therefore would generate invalid warnings.

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Don Zickus]

On Mon, Aug 09, 2010 at 10:07:40AM +0200, Heiko Carstens wrote:
> On Sun, Aug 08, 2010 at 09:58:42PM +0200, Frederic Weisbecker wrote:
> > > Maybe a better change would be to make it more generally available - right now 
> > > it's:
> > > 
> > >  config LOCKUP_DETECTOR
> > >          bool "Detect Hard and Soft Lockups"
> > >          depends on DEBUG_KERNEL && !S390
> > > 
> > > which means that it cannot be enabled when DEBUG_KERNEL is off.
> > > 
> > > So i think we should:
> > > 
> > >  - Remove the s390 hack and add an ARCH_HAS_LOCKUP_DETECTOR flag
> > 
> > 
> > 
> > If we do this, we'll need to add this config on every archs but s390.
> > We should better have ARCH_WANT_NO_LOCKUP_DETECTOR. I know that
> > "negative" meaning configs suck, but otherwise we would lose this
> > support on many archs.
> > 
> > Why s390 doesn't want the softlockup detector to begin with?
> 
> If I remember correctly then we disabled that back then because we got
> false positives. The reason for those were that the softlockup detector
> did not take steal time into account.
> E.g. if a guest cpu runs for 10 seconds, but the hypervisor would steal
> 9 seconds in order to run other guest cpus this specific cpu would still
> think it ran for 10 seconds and therefore would generate invalid warnings.

I have learned recently that is applies to all virtual machines including
KVM, Xen and VMWare(?).  However, you only see this when you overload the
hypervisor with lots of guests.  Which is why you normally don't see this
on those types of guests.

But any time based detection debug features (softlockup, hardlockup,
hung_task) could potentially run into this.

Cheers,
Don

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Heiko Carstens]

On Mon, Aug 09, 2010 at 09:49:17AM -0400, Don Zickus wrote:
> > > Why s390 doesn't want the softlockup detector to begin with?
> > 
> > If I remember correctly then we disabled that back then because we got
> > false positives. The reason for those were that the softlockup detector
> > did not take steal time into account.
> > E.g. if a guest cpu runs for 10 seconds, but the hypervisor would steal
> > 9 seconds in order to run other guest cpus this specific cpu would still
> > think it ran for 10 seconds and therefore would generate invalid warnings.
> 
> I have learned recently that is applies to all virtual machines including
> KVM, Xen and VMWare(?).  However, you only see this when you overload the
> hypervisor with lots of guests.  Which is why you normally don't see this
> on those types of guests.

On s390 you always run virtualized and usually even as a 2nd level guest.
Overloading a machine is quite common here.
The problem we have is that you can't tell afterwards if a warning was valid
or invalid due to overloading. Imho it is just pointless without taking steal
time into account and that's why we disabled it.

Re: [PATCH] lockup_detector: Make DETECT_HUNT_TASK default depend on LOCKUP_DETECTOR

[Li Zefan]

(Just came back from vacation)

>> Maybe a better change would be to make it more generally available - right now 
>> it's:
>>
>>  config LOCKUP_DETECTOR
>>          bool "Detect Hard and Soft Lockups"
>>          depends on DEBUG_KERNEL && !S390
>>
>> which means that it cannot be enabled when DEBUG_KERNEL is off.
>>
>> So i think we should:
>>
>>  - Remove the s390 hack and add an ARCH_HAS_LOCKUP_DETECTOR flag
> 
> If we do this, we'll need to add this config on every archs but s390.
> We should better have ARCH_WANT_NO_LOCKUP_DETECTOR. I know that
> "negative" meaning configs suck, but otherwise we would lose this
> support on many archs.
> 
> Why s390 doesn't want the softlockup detector to begin with?
> 
>>  - Remove the DEBUG_KERNEL dependency
> 
> Yeah.
> 

Seems we haven't come to an agreement yet? I don't know much about the
lockup detector code, so please take care of this. :)