[patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Ivan Kokshaysky]

Spinning the disks down across a 'halt' on Alpha is even
worse than doing that on reboot on i386 (assuming the
boot device is IDE disk).
Typically, the sequence to boot another kernel is:
# halt
kernel shuts down, firmware re-initializes,
then on firmware prompt we type something like
>>> boot -file new_kernel_image.gz

Unfortunately, the firmware does not expect the IDE drive
to be in standby mode and reports 'bootstrap failure' on
the first and all subsequent boot attempts until the
drive spins up, which is extremely annoying and
confuses users a lot.

This patch allows architectures override the default
behavior (don't spin the disks down on reboot only)
in asm/ide.h.

Ivan.

--- 2.6/drivers/ide/ide-disk.c	Fri May 21 21:06:12 2004
+++ linux/drivers/ide/ide-disk.c	Fri May 21 21:48:09 2004
@@ -1723,7 +1723,7 @@ static void ide_device_shutdown(struct d
 {
 	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
 
-	if (system_state == SYSTEM_RESTART) {
+	if (ide_shutdown_omit_standby(system_state)) {
 		ide_cacheflush_p(drive);
 		return;
 	}
--- 2.6/include/linux/ide.h	Fri May 21 21:47:13 2004
+++ linux/include/linux/ide.h	Fri May 21 21:48:09 2004
@@ -1709,4 +1709,8 @@ static inline int ata_can_queue(ide_driv
 
 extern struct bus_type ide_bus_type;
 
+#ifndef ide_shutdown_omit_standby
+#define ide_shutdown_omit_standby(sys_state)	(sys_state == SYSTEM_RESTART)
+#endif
+
 #endif /* _IDE_H */
--- 2.6/include/asm-alpha/ide.h	Fri May 21 21:47:54 2004
+++ linux/include/asm-alpha/ide.h	Fri May 21 21:48:34 2004
@@ -52,6 +52,8 @@ static inline unsigned long ide_default_
 #define ide_init_default_irq(base)	ide_default_irq(base)
 #endif
 
+#define ide_shutdown_omit_standby(sys_state)	(sys_state != SYSTEM_POWER_OFF)
+
 #include <asm-generic/ide_iops.h>
 
 #endif /* __KERNEL__ */

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Thu, 2004-05-27 at 17:49, Ivan Kokshaysky wrote:
> Spinning the disks down across a 'halt' on Alpha is even
> worse than doing that on reboot on i386 (assuming the
> boot device is IDE disk).
> Typically, the sequence to boot another kernel is:
> # halt
> kernel shuts down, firmware re-initializes,
> then on firmware prompt we type something like
> >>> boot -file new_kernel_image.gz
> 
> Unfortunately, the firmware does not expect the IDE drive
> to be in standby mode and reports 'bootstrap failure' on
> the first and all subsequent boot attempts until the
> drive spins up, which is extremely annoying and
> confuses users a lot.

how do you flush the disks' writecache then? Halting the disk seems to
be the only reliable way to do so.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Ivan Kokshaysky]

On Thu, May 27, 2004 at 06:26:33PM +0200, Arjan van de Ven wrote:
> how do you flush the disks' writecache then? Halting the disk seems to
> be the only reliable way to do so.

Isn't ide_cacheflush_p() supposed to do that on modern drives?

Ivan.

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

On Thu, May 27, 2004 at 09:11:13PM +0400, Ivan Kokshaysky wrote:
> On Thu, May 27, 2004 at 06:26:33PM +0200, Arjan van de Ven wrote:
> > how do you flush the disks' writecache then? Halting the disk seems to
> > be the only reliable way to do so.
> 
> Isn't ide_cacheflush_p() supposed to do that on modern drives?

that command seems to be optional to implement for drives unfortionately ;(

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Miquel van Smoorenburg]

In article <1085675193.7179.5.camel@laptop.fenrus.com>,
Arjan van de Ven  <arjanv@redhat.com> wrote:
>On Thu, 2004-05-27 at 17:49, Ivan Kokshaysky wrote:
>> Spinning the disks down across a 'halt' on Alpha is even
>> worse than doing that on reboot on i386 (assuming the
>> boot device is IDE disk).
>
>how do you flush the disks' writecache then? Halting the disk seems to
>be the only reliable way to do so.

On Alpha, Sun and other hardware, "halt" puts you into the firmware
monitor. It doesn't actually turn the machine off, so it's not
critical to flush the write cache of the disk - the only time
that is really neccesary is right before poweroff, in all other
cases the disk will expire the write cache itself.

Mike.

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Bartlomiej Zolnierkiewicz]

On Thursday 27 of May 2004 17:49, Ivan Kokshaysky wrote:
> Spinning the disks down across a 'halt' on Alpha is even
> worse than doing that on reboot on i386 (assuming the
> boot device is IDE disk).
> Typically, the sequence to boot another kernel is:
> # halt
> kernel shuts down, firmware re-initializes,
> then on firmware prompt we type something like
>
> >>> boot -file new_kernel_image.gz

How is it different from reboot?
[ I don't know much about alpha. ]

> Unfortunately, the firmware does not expect the IDE drive
> to be in standby mode and reports 'bootstrap failure' on
> the first and all subsequent boot attempts until the
> drive spins up, which is extremely annoying and
> confuses users a lot.

So how does it work in 2.4?

> This patch allows architectures override the default
> behavior (don't spin the disks down on reboot only)
> in asm/ide.h.

No more <asm/ide.h> crap, please.

> Ivan.
>
> --- 2.6/drivers/ide/ide-disk.c	Fri May 21 21:06:12 2004
> +++ linux/drivers/ide/ide-disk.c	Fri May 21 21:48:09 2004
> @@ -1723,7 +1723,7 @@ static void ide_device_shutdown(struct d
>  {
>  	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
>
> -	if (system_state == SYSTEM_RESTART) {
> +	if (ide_shutdown_omit_standby(system_state)) {
>  		ide_cacheflush_p(drive);
>  		return;
>  	}
> --- 2.6/include/linux/ide.h	Fri May 21 21:47:13 2004
> +++ linux/include/linux/ide.h	Fri May 21 21:48:09 2004
> @@ -1709,4 +1709,8 @@ static inline int ata_can_queue(ide_driv
>
>  extern struct bus_type ide_bus_type;
>
> +#ifndef ide_shutdown_omit_standby
> +#define ide_shutdown_omit_standby(sys_state)	(sys_state == SYSTEM_RESTART)
> +#endif
> +
>  #endif /* _IDE_H */
> --- 2.6/include/asm-alpha/ide.h	Fri May 21 21:47:54 2004
> +++ linux/include/asm-alpha/ide.h	Fri May 21 21:48:34 2004
> @@ -52,6 +52,8 @@ static inline unsigned long ide_default_
>  #define ide_init_default_irq(base)	ide_default_irq(base)
>  #endif
>
> +#define ide_shutdown_omit_standby(sys_state)	(sys_state !=
> SYSTEM_POWER_OFF) +
>  #include <asm-generic/ide_iops.h>
>
>  #endif /* __KERNEL__ */

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Ivan Kokshaysky]

On Thu, May 27, 2004 at 06:54:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > >>> boot -file new_kernel_image.gz
> 
> How is it different from reboot?

In this scenario it's not different, except you boot
to another kernel.

> So how does it work in 2.4?

2.4 needs similar fix.

> No more <asm/ide.h> crap, please.

Would #ifdef __alpha__ be better?

Ivan.

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Bartlomiej Zolnierkiewicz]

On Thursday 27 of May 2004 19:08, Ivan Kokshaysky wrote:
> On Thu, May 27, 2004 at 06:54:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > >>> boot -file new_kernel_image.gz
> >
> > How is it different from reboot?
>
> In this scenario it's not different, except you boot
> to another kernel.

Eh.

> > So how does it work in 2.4?
>
> 2.4 needs similar fix.
>
> > No more <asm/ide.h> crap, please.
>
> Would #ifdef __alpha__ be better?

actually CONFIG_ALPHA + comment why it is needed

Thanks,
Bartlomiej

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Ivan Kokshaysky]

On Thu, May 27, 2004 at 07:40:49PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Would #ifdef __alpha__ be better?
> 
> actually CONFIG_ALPHA + comment why it is needed

Ok, but now I'm thinking of probably better solution:
consider sys_state == SYSTEM_HALT as another special case
where we put the disk into standby mode *only* if it does
have write cache but doesn't suppurt cache flush command
(or cache flush fails for some reason).

Hopefully, most drives won't be spun down on halt.
I think that it should be acceptable for x86 as well.

Ivan.

--- 2.6/drivers/ide/ide-disk.c	Fri May 28 17:46:35 2004
+++ linux/drivers/ide/ide-disk.c	Fri May 28 18:42:47 2004
@@ -1686,13 +1686,19 @@ static void idedisk_setup (ide_drive_t *
 #endif
 }
 
-static void ide_cacheflush_p(ide_drive_t *drive)
+static int ide_cacheflush_p(ide_drive_t *drive)
 {
-	if (!drive->wcache || !ide_id_has_flush_cache(drive->id))
-		return;
+	if (!drive->wcache)
+		return 0;
+
+	if (!ide_id_has_flush_cache(drive->id))
+		return 1;
 
-	if (do_idedisk_flushcache(drive))
-		printk(KERN_INFO "%s: wcache flush failed!\n", drive->name);
+	if (!do_idedisk_flushcache(drive))
+		return 0;
+
+	printk(KERN_INFO "%s: wcache flush failed!\n", drive->name);
+	return 1;
 }
 
 static int idedisk_cleanup (ide_drive_t *drive)
@@ -1713,10 +1719,16 @@ static void ide_device_shutdown(struct d
 {
 	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
 
+	/* Never spin the disk down on reboot. */
 	if (system_state == SYSTEM_RESTART) {
 		ide_cacheflush_p(drive);
 		return;
 	}
+
+	/* Spin the disk down on halt only if attempt to flush the
+	   write cache fails. */
+	if (system_state == SYSTEM_HALT && !ide_cacheflush_p(drive))
+		return;
 
 	printk("Shutdown: %s\n", drive->name);
 	dev->bus->suspend(dev, PM_SUSPEND_STANDBY);

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Bartlomiej Zolnierkiewicz]

On Friday 28 of May 2004 17:10, Ivan Kokshaysky wrote:
> On Thu, May 27, 2004 at 07:40:49PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > Would #ifdef __alpha__ be better?
> >
> > actually CONFIG_ALPHA + comment why it is needed
>
> Ok, but now I'm thinking of probably better solution:
> consider sys_state == SYSTEM_HALT as another special case
> where we put the disk into standby mode *only* if it does
> have write cache but doesn't suppurt cache flush command
> (or cache flush fails for some reason).
>
> Hopefully, most drives won't be spun down on halt.
> I think that it should be acceptable for x86 as well.

- AFAIR there are some buggy disks having flush cache
  bits that need standby anyway

- you will hit 'halt problem' on alpha if your disk has
  write cache enabled and it doesn't have flush cache bits

-EAGAIN ;)

> Ivan.
>
> --- 2.6/drivers/ide/ide-disk.c	Fri May 28 17:46:35 2004
> +++ linux/drivers/ide/ide-disk.c	Fri May 28 18:42:47 2004
> @@ -1686,13 +1686,19 @@ static void idedisk_setup (ide_drive_t *
>  #endif
>  }
>
> -static void ide_cacheflush_p(ide_drive_t *drive)
> +static int ide_cacheflush_p(ide_drive_t *drive)
>  {
> -	if (!drive->wcache || !ide_id_has_flush_cache(drive->id))
> -		return;
> +	if (!drive->wcache)
> +		return 0;
> +
> +	if (!ide_id_has_flush_cache(drive->id))
> +		return 1;
>
> -	if (do_idedisk_flushcache(drive))
> -		printk(KERN_INFO "%s: wcache flush failed!\n", drive->name);
> +	if (!do_idedisk_flushcache(drive))
> +		return 0;
> +
> +	printk(KERN_INFO "%s: wcache flush failed!\n", drive->name);
> +	return 1;
>  }
>
>  static int idedisk_cleanup (ide_drive_t *drive)
> @@ -1713,10 +1719,16 @@ static void ide_device_shutdown(struct d
>  {
>  	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
>
> +	/* Never spin the disk down on reboot. */
>  	if (system_state == SYSTEM_RESTART) {
>  		ide_cacheflush_p(drive);
>  		return;
>  	}
> +
> +	/* Spin the disk down on halt only if attempt to flush the
> +	   write cache fails. */
> +	if (system_state == SYSTEM_HALT && !ide_cacheflush_p(drive))
> +		return;
>
>  	printk("Shutdown: %s\n", drive->name);
>  	dev->bus->suspend(dev, PM_SUSPEND_STANDBY);

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Ivan Kokshaysky]

On Fri, May 28, 2004 at 05:40:50PM +0200, Bartlomiej Zolnierkiewicz wrote:
> - AFAIR there are some buggy disks having flush cache
>   bits that need standby anyway

Oh horror. I wouldn't be surprised if some drives don't flush
the cache even on standby...

> - you will hit 'halt problem' on alpha if your disk has
>   write cache enabled and it doesn't have flush cache bits
> 
> -EAGAIN ;)

Yep... :-(

Here's variant of the first patch with CONFIG_ALPHA and a Very Big Comment.

Ivan.

--- 2.6/drivers/ide/ide-disk.c	Sat May 29 00:43:41 2004
+++ linux/drivers/ide/ide-disk.c	Sat May 29 00:43:06 2004
@@ -1713,7 +1713,22 @@ static void ide_device_shutdown(struct d
 {
 	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
 
+#ifdef	CONFIG_ALPHA
+	/* On Alpha, halt(8) doesn't actually turn the machine off,
+	   it puts you into the sort of firmware monitor. Typically,
+	   it's used to boot another kernel image, so it's not much
+	   different from reboot(8). Therefore, we don't need to
+	   spin down the disk in this case, especially since Alpha
+	   firmware doesn't handle disks in standby mode properly.
+	   On the other hand, it's reasonably safe to turn the power
+	   off when the shutdown process reaches the firmware prompt,
+	   as the firmware initialization takes rather long time -
+	   at least 10 seconds, which should be sufficient for
+	   the disk to expire its write cache. */
+	if (system_state != SYSTEM_POWER_OFF) {
+#else
 	if (system_state == SYSTEM_RESTART) {
+#endif
 		ide_cacheflush_p(drive);
 		return;
 	}

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Bartlomiej Zolnierkiewicz]

On Friday 28 of May 2004 23:37, Ivan Kokshaysky wrote:
> On Fri, May 28, 2004 at 05:40:50PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > - AFAIR there are some buggy disks having flush cache
> >   bits that need standby anyway
>
> Oh horror. I wouldn't be surprised if some drives don't flush
> the cache even on standby...
>
> > - you will hit 'halt problem' on alpha if your disk has
> >   write cache enabled and it doesn't have flush cache bits
> >
> > -EAGAIN ;)
>
> Yep... :-(
>
> Here's variant of the first patch with CONFIG_ALPHA and a Very Big Comment.

Looks good, thanks!

> Ivan.
>
> --- 2.6/drivers/ide/ide-disk.c	Sat May 29 00:43:41 2004
> +++ linux/drivers/ide/ide-disk.c	Sat May 29 00:43:06 2004
> @@ -1713,7 +1713,22 @@ static void ide_device_shutdown(struct d
>  {
>  	ide_drive_t *drive = container_of(dev, ide_drive_t, gendev);
>
> +#ifdef	CONFIG_ALPHA
> +	/* On Alpha, halt(8) doesn't actually turn the machine off,
> +	   it puts you into the sort of firmware monitor. Typically,
> +	   it's used to boot another kernel image, so it's not much
> +	   different from reboot(8). Therefore, we don't need to
> +	   spin down the disk in this case, especially since Alpha
> +	   firmware doesn't handle disks in standby mode properly.
> +	   On the other hand, it's reasonably safe to turn the power
> +	   off when the shutdown process reaches the firmware prompt,
> +	   as the firmware initialization takes rather long time -
> +	   at least 10 seconds, which should be sufficient for
> +	   the disk to expire its write cache. */
> +	if (system_state != SYSTEM_POWER_OFF) {
> +#else
>  	if (system_state == SYSTEM_RESTART) {
> +#endif
>  		ide_cacheflush_p(drive);
>  		return;
>  	}

Re: [patch 2.6] don't put IDE disks in standby mode on halt on Alpha

[Mark McClelland]

Ivan Kokshaysky wrote:
> Spinning the disks down across a 'halt' on Alpha is even
> worse than doing that on reboot on i386 (assuming the
> boot device is IDE disk).
> Typically, the sequence to boot another kernel is:
> # halt
> kernel shuts down, firmware re-initializes,
> then on firmware prompt we type something like
> 
>>>>boot -file new_kernel_image.gz
> 
> 
> Unfortunately, the firmware does not expect the IDE drive
> to be in standby mode and reports 'bootstrap failure' on
> the first and all subsequent boot attempts until the
> drive spins up, which is extremely annoying and
> confuses users a lot.
> 
> This patch allows architectures override the default
> behavior (don't spin the disks down on reboot only)
> in asm/ide.h.

This isn't completely safe. I sometimes power off my Alpha after 
rebooting to the monitor program, for various reasons.

Wouldn't it be better to spin the disks down like normal, but then spin 
them back up just before rebooting?

-- 
Mark McClelland
mark@alpha.dyndns.org