Re: [PATCH v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 1:15 AM, Ingo Molnar wrote: > > * Andrew Morton wrote: > >> On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron >> wrote: >> >> > The panic_timeout value can be set via the command line option 'panic=x', >> > or via >> > /proc/sys/kernel/panic, however that is not sufficient when the panic >> > occurs >> > before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT >> > so that we can set the desired value from the .config. >> > >> > The default panic_timeout value continues to be 0 - wait forever, except >> > for >> > powerpc and mips, which have been defaulted to 180 and 5 respectively. This >> > is in keeping with the fact that these arches already set panic_timeout in >> > their arch init code. However, I found three exceptions- two in mips and >> > one in >> > powerpc where the settings didn't match these default values. In those >> > cases, I >> > left the arch code so it continues to override, in case the user has not >> > changed >> > from the default. It would nice if these arches had one default value, or >> > if we >> > could determine the correct setting at compile-time. >> >> Felipe is proposing a simpler patch ("panic: setup panic_timeout >> early") which switches to early_param(). Is that sufficient for the >> (undescribed!) failure which you are presumably observing? > > Also note that that patch is still incomplete: if panic_timeout is > switched to early_param() then closely related functionality such as > pause_on_oops should be moved early as well... If that was the case, then whomever made the "oops" param generic sent an incomplete patch because it's also early_param, and it's interesting why you didn't have that problem with that patch, but you do with this one. -- Felipe Contreras -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 1:15 AM, Ingo Molnar mi...@kernel.org wrote: * Andrew Morton a...@linux-foundation.org wrote: On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron jba...@akamai.com wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. Felipe is proposing a simpler patch (panic: setup panic_timeout early) which switches to early_param(). Is that sufficient for the (undescribed!) failure which you are presumably observing? Also note that that patch is still incomplete: if panic_timeout is switched to early_param() then closely related functionality such as pause_on_oops should be moved early as well... If that was the case, then whomever made the oops param generic sent an incomplete patch because it's also early_param, and it's interesting why you didn't have that problem with that patch, but you do with this one. -- Felipe Contreras -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 05:04:14PM -0500, Jason Baron wrote: > On 11/19/2013 02:09 AM, Ingo Molnar wrote: > > > > * Jason Baron wrote: > > > >> On 11/18/2013 05:30 PM, Andrew Morton wrote: > >>> On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron > >>> wrote: > >>> > The panic_timeout value can be set via the command line option > 'panic=x', or via > /proc/sys/kernel/panic, however that is not sufficient when the panic > occurs > before we are able to set up these values. Thus, add a > CONFIG_PANIC_TIMEOUT > so that we can set the desired value from the .config. > > The default panic_timeout value continues to be 0 - wait forever, > except for powerpc and mips, which have been defaulted to 180 and > 5 respectively. This is in keeping with the fact that these > arches already set panic_timeout in their arch init code. > However, I found three exceptions- two in mips and one in powerpc > where the settings didn't match these default values. In those > cases, I left the arch code so it continues to override, in case > the user has not changed from the default. It would nice if these > arches had one default value, or if we could determine the > correct setting at compile-time. ... > > Sure, I can round up all the related patches in this area that make > sense and re-submit as a series. > > Felipe, would the CONFIG_PANIC_TIMEOUT=xx .config parameter work for your > needs, or would you still like to see the command-line processing moved > up? > > I'd also like to hear from the PowerPC folks about the arch defaults > there. Now, that mips is ok with CONFIG_PANIC_TIMEOUT, PowerPC is the > only arch doing specific initialization of 'panic_timeout'. Hi Jason, I think we'd like to choose the value at runtime, as we do now. The powerpc arch supports a wide spread of different hardware, so it's nice to be able to customise the value based on the platform. Also we build a single kernel that boots on many platforms, and so we can't pick the value at compile time. cheers -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 10:02:11AM +0100, Ralf Baechle wrote: > On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: > > > The panic_timeout value can be set via the command line option 'panic=x', > > or via > > /proc/sys/kernel/panic, however that is not sufficient when the panic occurs > > before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT > > so that we can set the desired value from the .config. > > > > The default panic_timeout value continues to be 0 - wait forever, except for > > powerpc and mips, which have been defaulted to 180 and 5 respectively. This > > is in keeping with the fact that these arches already set panic_timeout in > > their arch init code. However, I found three exceptions- two in mips and > > one in > > powerpc where the settings didn't match these default values. In those > > cases, I > > left the arch code so it continues to override, in case the user has not > > changed > > from the default. It would nice if these arches had one default value, or > > if we > > could determine the correct setting at compile-time. > > It's more complicated - MIPS was using the global default with five MIPS > platforms overriding the default. > > I propose to kill these overrides for sanity unless somebody comes up > with a good argument. Patch below. > > Ralf > > Signed-off-by: Ralf Baechle > > arch/mips/ar7/setup.c | 1 - > arch/mips/emma/markeins/setup.c | 3 --- > arch/mips/netlogic/xlp/setup.c | 1 - > arch/mips/netlogic/xlr/setup.c | 1 - > arch/mips/sibyte/swarm/setup.c | 2 -- > 5 files changed, 8 deletions(-) > > diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c > index 9a357ff..820b7a3 100644 > --- a/arch/mips/ar7/setup.c > +++ b/arch/mips/ar7/setup.c > @@ -92,7 +92,6 @@ void __init plat_mem_setup(void) > _machine_restart = ar7_machine_restart; > _machine_halt = ar7_machine_halt; > pm_power_off = ar7_machine_power_off; > - panic_timeout = 3; > > io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); > if (!io_base) > diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c > index d710058..9100122 100644 > --- a/arch/mips/emma/markeins/setup.c > +++ b/arch/mips/emma/markeins/setup.c > @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) > iomem_resource.start = EMMA2RH_IO_BASE; > iomem_resource.end = EMMA2RH_ROM_BASE - 1; > > - /* Reboot on panic */ > - panic_timeout = 180; > - > markeins_sio_setup(); > } > > diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c > index 6d981bb..54e75c7 100644 > --- a/arch/mips/netlogic/xlp/setup.c > +++ b/arch/mips/netlogic/xlp/setup.c > @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) > > void __init plat_mem_setup(void) > { > - panic_timeout = 5; > _machine_restart = (void (*)(char *))nlm_linux_exit; > _machine_halt = nlm_linux_exit; > pm_power_off= nlm_linux_exit; > diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c > index 214d123..921be5f 100644 > --- a/arch/mips/netlogic/xlr/setup.c > +++ b/arch/mips/netlogic/xlr/setup.c > @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) > > void __init plat_mem_setup(void) > { > - panic_timeout = 5; > _machine_restart = (void (*)(char *))nlm_linux_exit; > _machine_halt = nlm_linux_exit; > pm_power_off= nlm_linux_exit; > diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c > index 41707a2..3462c83 100644 > --- a/arch/mips/sibyte/swarm/setup.c > +++ b/arch/mips/sibyte/swarm/setup.c > @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) > #error invalid SiByte board configuration > #endif > > - panic_timeout = 5; /* For debug. */ > - > board_be_handler = swarm_be_handler; > > if (xicor_probe()) Acked-by: Jayachandran C For the arch/mips/netlogic changes, thanks for fixing this. JC. -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 10:02:11AM +0100, Ralf Baechle wrote: On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. Ralf Signed-off-by: Ralf Baechle r...@linux-mips.org arch/mips/ar7/setup.c | 1 - arch/mips/emma/markeins/setup.c | 3 --- arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- 5 files changed, 8 deletions(-) diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c index 9a357ff..820b7a3 100644 --- a/arch/mips/ar7/setup.c +++ b/arch/mips/ar7/setup.c @@ -92,7 +92,6 @@ void __init plat_mem_setup(void) _machine_restart = ar7_machine_restart; _machine_halt = ar7_machine_halt; pm_power_off = ar7_machine_power_off; - panic_timeout = 3; io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); if (!io_base) diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..9100122 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) iomem_resource.start = EMMA2RH_IO_BASE; iomem_resource.end = EMMA2RH_ROM_BASE - 1; - /* Reboot on panic */ - panic_timeout = 180; - markeins_sio_setup(); } diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c index 6d981bb..54e75c7 100644 --- a/arch/mips/netlogic/xlp/setup.c +++ b/arch/mips/netlogic/xlp/setup.c @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c index 214d123..921be5f 100644 --- a/arch/mips/netlogic/xlr/setup.c +++ b/arch/mips/netlogic/xlr/setup.c @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c index 41707a2..3462c83 100644 --- a/arch/mips/sibyte/swarm/setup.c +++ b/arch/mips/sibyte/swarm/setup.c @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) #error invalid SiByte board configuration #endif - panic_timeout = 5; /* For debug. */ - board_be_handler = swarm_be_handler; if (xicor_probe()) Acked-by: Jayachandran C jchan...@broadcom.com For the arch/mips/netlogic changes, thanks for fixing this. JC. -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 05:04:14PM -0500, Jason Baron wrote: On 11/19/2013 02:09 AM, Ingo Molnar wrote: * Jason Baron jba...@akamai.com wrote: On 11/18/2013 05:30 PM, Andrew Morton wrote: On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron jba...@akamai.com wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. ... Sure, I can round up all the related patches in this area that make sense and re-submit as a series. Felipe, would the CONFIG_PANIC_TIMEOUT=xx .config parameter work for your needs, or would you still like to see the command-line processing moved up? I'd also like to hear from the PowerPC folks about the arch defaults there. Now, that mips is ok with CONFIG_PANIC_TIMEOUT, PowerPC is the only arch doing specific initialization of 'panic_timeout'. Hi Jason, I think we'd like to choose the value at runtime, as we do now. The powerpc arch supports a wide spread of different hardware, so it's nice to be able to customise the value based on the platform. Also we build a single kernel that boots on many platforms, and so we can't pick the value at compile time. cheers -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 12:11:29PM -0500, Jason Baron wrote: > > I propose to kill these overrides for sanity unless somebody comes up > > with a good argument. Patch below. > > > > And so have the mips default be 0? IE drop the arch/mips/Kconfig bits from > the patch I posted? (Which could of course be configured to a non-zero value > by the user, if desired.) Yes. Ralf -- 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 v2] panic: Make panic_timeout configurable
On 11/19/2013 04:02 AM, Ralf Baechle wrote: > On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: > >> The panic_timeout value can be set via the command line option 'panic=x', or >> via >> /proc/sys/kernel/panic, however that is not sufficient when the panic occurs >> before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT >> so that we can set the desired value from the .config. >> >> The default panic_timeout value continues to be 0 - wait forever, except for >> powerpc and mips, which have been defaulted to 180 and 5 respectively. This >> is in keeping with the fact that these arches already set panic_timeout in >> their arch init code. However, I found three exceptions- two in mips and one >> in >> powerpc where the settings didn't match these default values. In those >> cases, I >> left the arch code so it continues to override, in case the user has not >> changed >> from the default. It would nice if these arches had one default value, or if >> we >> could determine the correct setting at compile-time. > > It's more complicated - MIPS was using the global default with five MIPS > platforms overriding the default. > > I propose to kill these overrides for sanity unless somebody comes up > with a good argument. Patch below. > And so have the mips default be 0? IE drop the arch/mips/Kconfig bits from the patch I posted? (Which could of course be configured to a non-zero value by the user, if desired.) Thanks, -Jason -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 11:51:27PM +0900, Shinya Kuribayashi wrote: > IIRC we had set it to 180 seconds for some historical reasons, but > I'm afraid nobody can recall the reason why it's set so in 2013... > Anyway I was thinking it too long and reduced to a few seconds locally > when debugging, so there shouldn't be a problem with this change. I think one motivation was to have boards reboot automatically avoiding the need to walk over to the board just powercycle or reset it - let alone reseting by emailing/phoning an admin! But that's a development-specific motivation and it shouldn't result in a hardcoded reboot timeout for everybody. Ralf -- 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 v2] panic: Make panic_timeout configurable
On 11/19/13 6:02 PM, Ralf Baechle wrote:> On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. Ralf Signed-off-by: Ralf Baechle arch/mips/ar7/setup.c | 1 - arch/mips/emma/markeins/setup.c | 3 --- arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- 5 files changed, 8 deletions(-) [...] diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..9100122 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) iomem_resource.start = EMMA2RH_IO_BASE; iomem_resource.end = EMMA2RH_ROM_BASE - 1; - /* Reboot on panic */ - panic_timeout = 180; - markeins_sio_setup(); } IIRC we had set it to 180 seconds for some historical reasons, but I'm afraid nobody can recall the reason why it's set so in 2013... Anyway I was thinking it too long and reduced to a few seconds locally when debugging, so there shouldn't be a problem with this change. FWIW, for EMMA2RH portion: Acked-by: Shinya Kuribayashi Thank you always for your help, Ralf. -- 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 v2] panic: Make panic_timeout configurable
On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: > The panic_timeout value can be set via the command line option 'panic=x', or > via > /proc/sys/kernel/panic, however that is not sufficient when the panic occurs > before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT > so that we can set the desired value from the .config. > > The default panic_timeout value continues to be 0 - wait forever, except for > powerpc and mips, which have been defaulted to 180 and 5 respectively. This > is in keeping with the fact that these arches already set panic_timeout in > their arch init code. However, I found three exceptions- two in mips and one > in > powerpc where the settings didn't match these default values. In those cases, > I > left the arch code so it continues to override, in case the user has not > changed > from the default. It would nice if these arches had one default value, or if > we > could determine the correct setting at compile-time. It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. Ralf Signed-off-by: Ralf Baechle arch/mips/ar7/setup.c | 1 - arch/mips/emma/markeins/setup.c | 3 --- arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- 5 files changed, 8 deletions(-) diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c index 9a357ff..820b7a3 100644 --- a/arch/mips/ar7/setup.c +++ b/arch/mips/ar7/setup.c @@ -92,7 +92,6 @@ void __init plat_mem_setup(void) _machine_restart = ar7_machine_restart; _machine_halt = ar7_machine_halt; pm_power_off = ar7_machine_power_off; - panic_timeout = 3; io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); if (!io_base) diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..9100122 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) iomem_resource.start = EMMA2RH_IO_BASE; iomem_resource.end = EMMA2RH_ROM_BASE - 1; - /* Reboot on panic */ - panic_timeout = 180; - markeins_sio_setup(); } diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c index 6d981bb..54e75c7 100644 --- a/arch/mips/netlogic/xlp/setup.c +++ b/arch/mips/netlogic/xlp/setup.c @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c index 214d123..921be5f 100644 --- a/arch/mips/netlogic/xlr/setup.c +++ b/arch/mips/netlogic/xlr/setup.c @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c index 41707a2..3462c83 100644 --- a/arch/mips/sibyte/swarm/setup.c +++ b/arch/mips/sibyte/swarm/setup.c @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) #error invalid SiByte board configuration #endif - panic_timeout = 5; /* For debug. */ - board_be_handler = swarm_be_handler; if (xicor_probe()) -- 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 v2] panic: Make panic_timeout configurable
On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. Ralf Signed-off-by: Ralf Baechle r...@linux-mips.org arch/mips/ar7/setup.c | 1 - arch/mips/emma/markeins/setup.c | 3 --- arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- 5 files changed, 8 deletions(-) diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c index 9a357ff..820b7a3 100644 --- a/arch/mips/ar7/setup.c +++ b/arch/mips/ar7/setup.c @@ -92,7 +92,6 @@ void __init plat_mem_setup(void) _machine_restart = ar7_machine_restart; _machine_halt = ar7_machine_halt; pm_power_off = ar7_machine_power_off; - panic_timeout = 3; io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); if (!io_base) diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..9100122 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) iomem_resource.start = EMMA2RH_IO_BASE; iomem_resource.end = EMMA2RH_ROM_BASE - 1; - /* Reboot on panic */ - panic_timeout = 180; - markeins_sio_setup(); } diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c index 6d981bb..54e75c7 100644 --- a/arch/mips/netlogic/xlp/setup.c +++ b/arch/mips/netlogic/xlp/setup.c @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c index 214d123..921be5f 100644 --- a/arch/mips/netlogic/xlr/setup.c +++ b/arch/mips/netlogic/xlr/setup.c @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c index 41707a2..3462c83 100644 --- a/arch/mips/sibyte/swarm/setup.c +++ b/arch/mips/sibyte/swarm/setup.c @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) #error invalid SiByte board configuration #endif - panic_timeout = 5; /* For debug. */ - board_be_handler = swarm_be_handler; if (xicor_probe()) -- 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 v2] panic: Make panic_timeout configurable
On 11/19/13 6:02 PM, Ralf Baechle wrote: On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. Ralf Signed-off-by: Ralf Baechle r...@linux-mips.org arch/mips/ar7/setup.c | 1 - arch/mips/emma/markeins/setup.c | 3 --- arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- 5 files changed, 8 deletions(-) [...] diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..9100122 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -111,9 +111,6 @@ void __init plat_mem_setup(void) iomem_resource.start = EMMA2RH_IO_BASE; iomem_resource.end = EMMA2RH_ROM_BASE - 1; - /* Reboot on panic */ - panic_timeout = 180; - markeins_sio_setup(); } IIRC we had set it to 180 seconds for some historical reasons, but I'm afraid nobody can recall the reason why it's set so in 2013... Anyway I was thinking it too long and reduced to a few seconds locally when debugging, so there shouldn't be a problem with this change. FWIW, for EMMA2RH portion: Acked-by: Shinya Kuribayashi skuri...@pobox.com Thank you always for your help, Ralf. -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 11:51:27PM +0900, Shinya Kuribayashi wrote: IIRC we had set it to 180 seconds for some historical reasons, but I'm afraid nobody can recall the reason why it's set so in 2013... Anyway I was thinking it too long and reduced to a few seconds locally when debugging, so there shouldn't be a problem with this change. I think one motivation was to have boards reboot automatically avoiding the need to walk over to the board just powercycle or reset it - let alone reseting by emailing/phoning an admin! But that's a development-specific motivation and it shouldn't result in a hardcoded reboot timeout for everybody. Ralf -- 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 v2] panic: Make panic_timeout configurable
On 11/19/2013 04:02 AM, Ralf Baechle wrote: On Mon, Nov 18, 2013 at 09:04:36PM +, Jason Baron wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. It's more complicated - MIPS was using the global default with five MIPS platforms overriding the default. I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. And so have the mips default be 0? IE drop the arch/mips/Kconfig bits from the patch I posted? (Which could of course be configured to a non-zero value by the user, if desired.) Thanks, -Jason -- 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 v2] panic: Make panic_timeout configurable
On Tue, Nov 19, 2013 at 12:11:29PM -0500, Jason Baron wrote: I propose to kill these overrides for sanity unless somebody comes up with a good argument. Patch below. And so have the mips default be 0? IE drop the arch/mips/Kconfig bits from the patch I posted? (Which could of course be configured to a non-zero value by the user, if desired.) Yes. Ralf -- 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 v2] panic: Make panic_timeout configurable
On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron wrote: > The panic_timeout value can be set via the command line option 'panic=x', or > via > /proc/sys/kernel/panic, however that is not sufficient when the panic occurs > before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT > so that we can set the desired value from the .config. > > The default panic_timeout value continues to be 0 - wait forever, except for > powerpc and mips, which have been defaulted to 180 and 5 respectively. This > is in keeping with the fact that these arches already set panic_timeout in > their arch init code. However, I found three exceptions- two in mips and one > in > powerpc where the settings didn't match these default values. In those cases, > I > left the arch code so it continues to override, in case the user has not > changed > from the default. It would nice if these arches had one default value, or if > we > could determine the correct setting at compile-time. Felipe is proposing a simpler patch ("panic: setup panic_timeout early") which switches to early_param(). Is that sufficient for the (undescribed!) failure which you are presumably observing? -- 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 v2] panic: Make panic_timeout configurable
The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. Signed-off-by: Jason Baron --- -Restrict arch defaults to arch/ code -add set_arch_panic_timeout(), in case .config specifies a non-default timeout arch/mips/Kconfig | 4 arch/mips/ar7/setup.c | 2 +- arch/mips/emma/markeins/setup.c| 2 +- arch/mips/include/asm/setup.h | 2 ++ arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- arch/powerpc/Kconfig | 4 arch/powerpc/include/asm/setup.h | 1 + arch/powerpc/kernel/setup_32.c | 3 --- arch/powerpc/kernel/setup_64.c | 3 --- arch/powerpc/platforms/pseries/setup.c | 2 +- include/linux/kernel.h | 9 + kernel/panic.c | 2 +- lib/Kconfig.debug | 11 +++ 15 files changed, 35 insertions(+), 14 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 650de39..0c21758 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -837,6 +837,10 @@ config SCHED_OMIT_FRAME_POINTER bool default y +config PANIC_TIMEOUT + int + default 5 + # # Select some configuration options automatically based on user selections. # diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c index 9a357ff..46e7abc 100644 --- a/arch/mips/ar7/setup.c +++ b/arch/mips/ar7/setup.c @@ -92,7 +92,7 @@ void __init plat_mem_setup(void) _machine_restart = ar7_machine_restart; _machine_halt = ar7_machine_halt; pm_power_off = ar7_machine_power_off; - panic_timeout = 3; + set_arch_panic_timeout(3, ARCH_PANIC_TIMEOUT); io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); if (!io_base) diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..03a47a7 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -112,7 +112,7 @@ void __init plat_mem_setup(void) iomem_resource.end = EMMA2RH_ROM_BASE - 1; /* Reboot on panic */ - panic_timeout = 180; + set_arch_panic_timeout(180, ARCH_PANIC_TIMEOUT); markeins_sio_setup(); } diff --git a/arch/mips/include/asm/setup.h b/arch/mips/include/asm/setup.h index d7bfdeb..9d1b53c 100644 --- a/arch/mips/include/asm/setup.h +++ b/arch/mips/include/asm/setup.h @@ -24,4 +24,6 @@ extern unsigned long ebase; extern void per_cpu_trap_init(bool); extern void cpu_cache_init(void); +#define ARCH_PANIC_TIMEOUT 5 + #endif /* __SETUP_H */ diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c index 6d981bb..54e75c7 100644 --- a/arch/mips/netlogic/xlp/setup.c +++ b/arch/mips/netlogic/xlp/setup.c @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c index 214d123..921be5f 100644 --- a/arch/mips/netlogic/xlr/setup.c +++ b/arch/mips/netlogic/xlr/setup.c @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c index 41707a2..3462c83 100644 --- a/arch/mips/sibyte/swarm/setup.c +++ b/arch/mips/sibyte/swarm/setup.c @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) #error invalid SiByte board configuration #endif - panic_timeout = 5; /* For debug. */ - board_be_handler = swarm_be_handler; if (xicor_probe()) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b44b52c..b2be8e8 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -147,6 +147,10 @@ config EARLY_PRINTK bool
[PATCH v2] panic: Make panic_timeout configurable
The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. Signed-off-by: Jason Baron jba...@akamai.com --- -Restrict arch defaults to arch/ code -add set_arch_panic_timeout(), in case .config specifies a non-default timeout arch/mips/Kconfig | 4 arch/mips/ar7/setup.c | 2 +- arch/mips/emma/markeins/setup.c| 2 +- arch/mips/include/asm/setup.h | 2 ++ arch/mips/netlogic/xlp/setup.c | 1 - arch/mips/netlogic/xlr/setup.c | 1 - arch/mips/sibyte/swarm/setup.c | 2 -- arch/powerpc/Kconfig | 4 arch/powerpc/include/asm/setup.h | 1 + arch/powerpc/kernel/setup_32.c | 3 --- arch/powerpc/kernel/setup_64.c | 3 --- arch/powerpc/platforms/pseries/setup.c | 2 +- include/linux/kernel.h | 9 + kernel/panic.c | 2 +- lib/Kconfig.debug | 11 +++ 15 files changed, 35 insertions(+), 14 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 650de39..0c21758 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -837,6 +837,10 @@ config SCHED_OMIT_FRAME_POINTER bool default y +config PANIC_TIMEOUT + int + default 5 + # # Select some configuration options automatically based on user selections. # diff --git a/arch/mips/ar7/setup.c b/arch/mips/ar7/setup.c index 9a357ff..46e7abc 100644 --- a/arch/mips/ar7/setup.c +++ b/arch/mips/ar7/setup.c @@ -92,7 +92,7 @@ void __init plat_mem_setup(void) _machine_restart = ar7_machine_restart; _machine_halt = ar7_machine_halt; pm_power_off = ar7_machine_power_off; - panic_timeout = 3; + set_arch_panic_timeout(3, ARCH_PANIC_TIMEOUT); io_base = (unsigned long)ioremap(AR7_REGS_BASE, 0x1); if (!io_base) diff --git a/arch/mips/emma/markeins/setup.c b/arch/mips/emma/markeins/setup.c index d710058..03a47a7 100644 --- a/arch/mips/emma/markeins/setup.c +++ b/arch/mips/emma/markeins/setup.c @@ -112,7 +112,7 @@ void __init plat_mem_setup(void) iomem_resource.end = EMMA2RH_ROM_BASE - 1; /* Reboot on panic */ - panic_timeout = 180; + set_arch_panic_timeout(180, ARCH_PANIC_TIMEOUT); markeins_sio_setup(); } diff --git a/arch/mips/include/asm/setup.h b/arch/mips/include/asm/setup.h index d7bfdeb..9d1b53c 100644 --- a/arch/mips/include/asm/setup.h +++ b/arch/mips/include/asm/setup.h @@ -24,4 +24,6 @@ extern unsigned long ebase; extern void per_cpu_trap_init(bool); extern void cpu_cache_init(void); +#define ARCH_PANIC_TIMEOUT 5 + #endif /* __SETUP_H */ diff --git a/arch/mips/netlogic/xlp/setup.c b/arch/mips/netlogic/xlp/setup.c index 6d981bb..54e75c7 100644 --- a/arch/mips/netlogic/xlp/setup.c +++ b/arch/mips/netlogic/xlp/setup.c @@ -92,7 +92,6 @@ static void __init xlp_init_mem_from_bars(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/netlogic/xlr/setup.c b/arch/mips/netlogic/xlr/setup.c index 214d123..921be5f 100644 --- a/arch/mips/netlogic/xlr/setup.c +++ b/arch/mips/netlogic/xlr/setup.c @@ -92,7 +92,6 @@ static void nlm_linux_exit(void) void __init plat_mem_setup(void) { - panic_timeout = 5; _machine_restart = (void (*)(char *))nlm_linux_exit; _machine_halt = nlm_linux_exit; pm_power_off= nlm_linux_exit; diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c index 41707a2..3462c83 100644 --- a/arch/mips/sibyte/swarm/setup.c +++ b/arch/mips/sibyte/swarm/setup.c @@ -134,8 +134,6 @@ void __init plat_mem_setup(void) #error invalid SiByte board configuration #endif - panic_timeout = 5; /* For debug. */ - board_be_handler = swarm_be_handler; if (xicor_probe()) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b44b52c..b2be8e8 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -147,6 +147,10 @@ config
Re: [PATCH v2] panic: Make panic_timeout configurable
On Mon, 18 Nov 2013 21:04:36 + (GMT) Jason Baron jba...@akamai.com wrote: The panic_timeout value can be set via the command line option 'panic=x', or via /proc/sys/kernel/panic, however that is not sufficient when the panic occurs before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT so that we can set the desired value from the .config. The default panic_timeout value continues to be 0 - wait forever, except for powerpc and mips, which have been defaulted to 180 and 5 respectively. This is in keeping with the fact that these arches already set panic_timeout in their arch init code. However, I found three exceptions- two in mips and one in powerpc where the settings didn't match these default values. In those cases, I left the arch code so it continues to override, in case the user has not changed from the default. It would nice if these arches had one default value, or if we could determine the correct setting at compile-time. Felipe is proposing a simpler patch (panic: setup panic_timeout early) which switches to early_param(). Is that sufficient for the (undescribed!) failure which you are presumably observing? -- 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/