* Re: [PATCH v2] panic: Make panic_timeout configurable
2013-11-19 9:02 ` [PATCH v2] panic: Make panic_timeout configurable Ralf Baechle
@ 2013-11-19 14:51 ` Shinya Kuribayashi
2013-11-19 16:38 ` Ralf Baechle
2013-11-19 17:11 ` Jason Baron
2013-11-21 8:44 ` Jayachandran C.
2 siblings, 1 reply; 6+ messages in thread
From: Shinya Kuribayashi @ 2013-11-19 14:51 UTC (permalink / raw)
To: ralf, jbaron
Cc: mingo, benh, paulus, akpm, linux-kernel, linux-mips, florian,
jchandra, ganesanr
On 11/19/13 6:02 PM, Ralf Baechle wrote:> On Mon, Nov 18, 2013 at 09:04:36PM +0000, 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 <ralf@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 <skuribay@pobox.com>
Thank you always for your help, Ralf.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] panic: Make panic_timeout configurable
2013-11-19 14:51 ` Shinya Kuribayashi
@ 2013-11-19 16:38 ` Ralf Baechle
0 siblings, 0 replies; 6+ messages in thread
From: Ralf Baechle @ 2013-11-19 16:38 UTC (permalink / raw)
To: Shinya Kuribayashi
Cc: jbaron, mingo, benh, paulus, akpm, linux-kernel, linux-mips,
florian, jchandra, ganesanr
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] panic: Make panic_timeout configurable
2013-11-19 9:02 ` [PATCH v2] panic: Make panic_timeout configurable Ralf Baechle
2013-11-19 14:51 ` Shinya Kuribayashi
@ 2013-11-19 17:11 ` Jason Baron
2013-11-19 17:22 ` Ralf Baechle
2013-11-21 8:44 ` Jayachandran C.
2 siblings, 1 reply; 6+ messages in thread
From: Jason Baron @ 2013-11-19 17:11 UTC (permalink / raw)
To: Ralf Baechle
Cc: mingo@kernel.org, benh@kernel.crashing.org, paulus@samba.org,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mips@linux-mips.org, Florian Fainelli, Shinya Kuribayashi,
Jayachandran C, Ganesan Ramalingam
On 11/19/2013 04:02 AM, Ralf Baechle wrote:
> On Mon, Nov 18, 2013 at 09:04:36PM +0000, 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] panic: Make panic_timeout configurable
2013-11-19 17:11 ` Jason Baron
@ 2013-11-19 17:22 ` Ralf Baechle
0 siblings, 0 replies; 6+ messages in thread
From: Ralf Baechle @ 2013-11-19 17:22 UTC (permalink / raw)
To: Jason Baron
Cc: mingo@kernel.org, benh@kernel.crashing.org, paulus@samba.org,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mips@linux-mips.org, Florian Fainelli, Shinya Kuribayashi,
Jayachandran C, Ganesan Ramalingam
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] panic: Make panic_timeout configurable
2013-11-19 9:02 ` [PATCH v2] panic: Make panic_timeout configurable Ralf Baechle
2013-11-19 14:51 ` Shinya Kuribayashi
2013-11-19 17:11 ` Jason Baron
@ 2013-11-21 8:44 ` Jayachandran C.
2 siblings, 0 replies; 6+ messages in thread
From: Jayachandran C. @ 2013-11-21 8:44 UTC (permalink / raw)
To: Ralf Baechle
Cc: Jason Baron, mingo, benh, paulus, akpm, linux-kernel, linux-mips,
Florian Fainelli, Shinya Kuribayashi, Ganesan Ramalingam
On Tue, Nov 19, 2013 at 10:02:11AM +0100, Ralf Baechle wrote:
> On Mon, Nov 18, 2013 at 09:04:36PM +0000, 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 <ralf@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, 0x10000);
> 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 <jchandra@broadcom.com>
For the arch/mips/netlogic changes, thanks for fixing this.
JC.
^ permalink raw reply [flat|nested] 6+ messages in thread