Linux MIPS Architecture development
 help / color / mirror / Atom feed
* Re: [PATCH v2] panic: Make panic_timeout configurable
       [not found] <20131118210436.233B5202A@prod-mail-relay06.akamai.com>
@ 2013-11-19  9:02 ` Ralf Baechle
  2013-11-19 14:51   ` Shinya Kuribayashi
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ralf Baechle @ 2013-11-19  9:02 UTC (permalink / raw)
  To: Jason Baron
  Cc: mingo, benh, paulus, akpm, linux-kernel, linux-mips,
	Florian Fainelli, Shinya Kuribayashi, Jayachandran C,
	Ganesan Ramalingam

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())

^ permalink raw reply related	[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 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

end of thread, other threads:[~2013-11-21  8:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20131118210436.233B5202A@prod-mail-relay06.akamai.com>
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-19 17:22     ` Ralf Baechle
2013-11-21  8:44   ` Jayachandran C.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox