PATCH: Shutdown IDE before powering off.
Hi.
Here's a patch Bernard Blackham posted to the Software Suspend mailing list, which has fixed data-not-being-properly flushed issues for some people. (Forwarded with Bernard's permission).
Regards,
Nigel
Signature · 2 lines
My work on Software Suspend is graciously brought to you by LinuxFund.org.
drivers/ide/ide.c renamed from drivers/ide/ide.c.orig +6 -0
@@ -2493,6 +2493,11 @@return 0 ; } +static void ide_drive_shutdown (struct device * dev) +{ +generic_ide_suspend (dev,5 ); +} +int ide_register_driver (ide_driver_t *driver) {struct list_head list;
@@ -2519,6 +2524,7 @@ driver->gen_driver.name = (char *) driver->name; driver->gen_driver.bus = &ide_bus_type; driver->gen_driver.remove = ide_drive_remove; + driver->gen_driver.shutdown = ide_drive_shutdown;return driver_register (&driver->gen_driver); }
Quoted 14 lines · Nigel Cunningham
+static void
ide_drive_shutdown(struct device * dev)
+{
+generic_ide_suspend(dev, 5);
+}
+
intide_register_driver(ide_driver_t *driver){
structlist_headlist;
@@ -2519,6 +2524,7 @@
driver->gen_driver.name= (char *)driver->name;driver->gen_driver.bus=&ide_bus_type;driver->gen_driver.remove=ide_drive_remove;
+driver->gen_driver.shutdown=ide_drive_shutdown;
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
Quote from Andrew Morton <[email protected]>:
Quoted 16 lines
Quoted 14 lines · Nigel Cunningham
+static void
ide_drive_shutdown(struct device * dev)
+{
+generic_ide_suspend(dev, 5);
+}
+
intide_register_driver(ide_driver_t *driver){
structlist_headlist;
@@ -2519,6 +2524,7 @@
driver->gen_driver.name= (char *)driver->name;driver->gen_driver.bus=&ide_bus_type;driver->gen_driver.remove=ide_drive_remove;
+driver->gen_driver.shutdown=ide_drive_shutdown;
Quoted 2 lines
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
I think it is an attempt to force some broken drives to flush their cache, but I wonder whether it will simply move the problem from one set of broken drives to another :-).
John.
Hi.
Quoted 6 lines · John Bradford
Quoted 2 lines
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
I think it is an attempt to force some broken drives to flush their cache, but I wonder whether it will simply move the problem from one set of broken drives to another :-).
Yes, they were trying to get caches flushed. If this attempt is misguided, that's fine. Is there a better way?
Regards,
Nigel
Signature · 2 lines
My work on Software Suspend is graciously brought to you by LinuxFund.org.
Quoted 7 lines · Nigel Cunningham
Hi.
Quoted 3 lines · John Bradford
Quoted 2 lines
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
Quoted 6 lines
Quoted 3 lines
I think it is an attempt to force some broken drives to flush their cache, but I wonder whether it will simply move the problem from one set of broken drives to another :-).
Yes, they were trying to get caches flushed. If this attempt is misguided, that's fine. Is there a better way?
A couple of thoughts come to mind:
a) Don't do it if the user typed reboot - only do it if we're powering down.
b) Try to do a cache flush instead. If that fails (do we know?) then power down the disk instead.
Hi.
Quoted 3 lines · Andrew Morton
A couple of thoughts come to mind:
a) Don't do it if the user typed reboot - only do it if we're powering down.
You'd think a parameter called shutdown would do that :>
Quoted 2 lines
b) Try to do a cache flush instead. If that fails (do we know?) then power down the disk instead.
We're calling drivers_suspend and then sys_reboot for powering down, and only calling sys_reboot when rebooting. Shouldn't cache flushes already be happening?
Regards,
Nigel
Signature · 2 lines
My work on Software Suspend is graciously brought to you by LinuxFund.org.
Quoted 17 lines · Andrew Morton
Quoted 7 lines · Nigel Cunningham
Hi.
Quoted 3 lines · John Bradford
Quoted 2 lines
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
Quoted 6 lines
Quoted 3 lines
I think it is an attempt to force some broken drives to flush their cache, but I wonder whether it will simply move the problem from one set of broken drives to another :-).
Yes, they were trying to get caches flushed. If this attempt is misguided, that's fine. Is there a better way?
Quoted 6 lines
A couple of thoughts come to mind:
a) Don't do it if the user typed reboot - only do it if we're powering down.
b) Try to do a cache flush instead. If that fails (do we know?) then power down the disk instead.
I'm either shock or very very worried that the reboot notifier that flushes IDE in 2.4.x, ide_notifier, is nowhere to be seen in 2.6.x :( That seems like the real problem -- the code _used_ to be there.
Jeff
Quoted 3 lines · Jeff Garzik
I'm either shock or very very worried that the reboot notifier that flushes IDE in 2.4.x,
ide_notifier, is nowhere to be seen in 2.6.x :( That seems like the real problem -- the code _used_ to be there.
Yep, it should be re-added. I wonder when/why it was removed?
--bart
Quote from Nigel Cunningham <[email protected]>:
Quoted 10 lines
--=-1L1FlHM683Yn00jU9UMu
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printableHi.
Quoted 2 lines · John Bradford
Quoted 2 lines
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
Quoted 7 lines
Quoted 4 lines
=20
I think it is an attempt to force some broken drives to flush their cache, but I wonder whether it will simply move the problem from one set of broken drives to another :-).Yes, they were trying to get caches flushed. If this attempt is misguided, that's fine. Is there a better way?
It was discussed at length around the 2.4.20 timeframe, when the power-off cache-flush and spin down behavior was changed, but I don't remember any real conclusion being reached.
John.
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"John" == John Bradford <[email protected]> writes:
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
John> I think it is an attempt to force some broken drives to flush John> their cache, but I wonder whether it will simply move the John> problem from one set of broken drives to another :-).
It will. I've had to work with a few drives or drive combos over the years that would not spin up reliably. It was vital to keep them spinning once they were (all) up. Adding this would make reboot unnecessarily unuseable in such cases. Perhaps just flush, pause, flush would work as well?
Or even the logical equivilent to sync;sync;sync;reboot?
-JimC
Well, as far as Suspend can see, all the data has been written. By the time we reach this point, the end_buffer_io_async routine has been called for every write submitted, and the last write submitted by anything else was at least a few seconds ago (all other processes are frozen and drivers suspended), so all data _should_ be on disk already and sync should do nothing. Actually, we wouldn't want to call sync anyway for reasons I won't go into here (unnecessary complication). I'm sure you'd agree that we would want to delay for an arbitrary length of time either (no guarantees that that would cut the mustard). We need to flush caches properly.
Regards,
Nigel
Quoted 19 lines · James H. Cloos Jr.
John> I think it is an attempt to force some broken drives to flush John> their cache, but I wonder whether it will simply move the John> problem from one set of broken drives to another :-).
It will. I've had to work with a few drives or drive combos over the years that would not spin up reliably. It was vital to keep them spinning once they were (all) up. Adding this would make reboot unnecessarily unuseable in such cases. Perhaps just flush, pause, flush would work as well?
Or even the logical equivilent to sync;sync;sync;reboot?
(quoted diff) +0
-2-JimC -To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 2 lines
My work on Software Suspend is graciously brought to you by LinuxFund.org.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Nigel" == Nigel Cunningham <[email protected]> writes:
Nigel> Actually, we wouldn't want to call sync
Nigel> anyway for reasons I won't go into here
Sorry for the confusion; I didn't mean call sync so much as flush synchronously (ie wait for the drive to ack) thrice before the reboot.
-JimC
Quoted 12 lines · James H. Cloos Jr.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"John" == John Bradford <[email protected]> writes:
This spins down the disk(s) when you're just doing do a reboot. That's fairly irritating and could affect reboot times if one has many disks.
John> I think it is an attempt to force some broken drives to flush John> their cache, but I wonder whether it will simply move the John> problem from one set of broken drives to another :-).
Quoted 5 lines
It will. I've had to work with a few drives or drive combos over the years that would not spin up reliably. It was vital to keep them spinning once they were (all) up. Adding this would make reboot unnecessarily unuseable in such cases. Perhaps just flush, pause, flush would work as well?
Flush is what is needed, flush is what it does in 2.4, and flush is what it should do in 2.6 :)
Rebooting does not shut down nor unload the IDE driver, so it is -critical- that a flush occurs before reboot, otherwise it is entirely possible that writes the drive has ack'd back to the OS will not actually get written to the media.
Jeff