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
--
My work on Software Suspend is graciously brought to you by
LinuxFund.org.
Nigel Cunningham <[email protected]> wrote:
>
> +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;
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]>:
> Nigel Cunningham <[email protected]> wrote:
> >
> > +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;
>
> 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.
On Thu, 2004-01-22 at 21:13, John Bradford wrote:
> > 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
--
My work on Software Suspend is graciously brought to you by
LinuxFund.org.
Nigel Cunningham <[email protected]> wrote:
>
> Hi.
>
> On Thu, 2004-01-22 at 21:13, John Bradford wrote:
> > > 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?
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.
On Thu, 2004-01-22 at 21:45, Andrew Morton wrote:
> 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 :>
> 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
--
My work on Software Suspend is graciously brought to you by
LinuxFund.org.
Quote from Nigel Cunningham <[email protected]>:
>
> --=-1L1FlHM683Yn00jU9UMu
> Content-Type: text/plain
> Content-Transfer-Encoding: quoted-printable
>
> Hi.
>
> On Thu, 2004-01-22 at 21:13, John Bradford wrote:
> > > 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.
> >=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.
>>>>> "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
On Thursday 22 of January 2004 17:02, Jeff Garzik wrote:
> 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
James H. Cloos Jr. wrote:
>>>>>>"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?
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
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
On Fri, 2004-01-23 at 08:19, James H. Cloos Jr. wrote:
> 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
>
> -
> 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/
--
My work on Software Suspend is graciously brought to you by
LinuxFund.org.
Andrew Morton wrote:
> Nigel Cunningham <[email protected]> wrote:
>
>>Hi.
>>
>>On Thu, 2004-01-22 at 21:13, John Bradford wrote:
>>
>>>>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?
>
>
> 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
>>>>> "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
Thus wrote Bartlomiej Zolnierkiewicz:
>
> On Thursday 22 of January 2004 17:02, Jeff Garzik wrote:
> > 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?
Hi,
What's the current status of this issue?
Best regards,
--
Karol 'sziwan' Kozimor
[email protected]
On Fri, 2004-03-12 at 08:46, Karol Kozimor wrote:
> Thus wrote Bartlomiej Zolnierkiewicz:
> >
> > On Thursday 22 of January 2004 17:02, Jeff Garzik wrote:
> > > 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?
Ideally, it should use the same mecanism as the PM requests...
In fact, the shutdown is just a special case of PM request. I think
ultimately, we should drop the various "shutdown()" functions in the
drivers in favor of a "state" selector for PM. That goes along with
the current problem of "state" in PM beeing completely bogus. The
constants defined by linux/pm.h are in no way related to what
the various drivers have come to expect.
enum {
PM_SUSPEND_ON,
PM_SUSPEND_STANDBY,
PM_SUSPEND_MEM,
PM_SUSPEND_DISK,
PM_SUSPEND_MAX,
};
Which basically gives is MEM=2 and DISK=3, while drivers usually
expect MEM=3 and DISK=4 while nobody really cares about 2 except
some specific stuffs in the arch code (or radeonfb on pmacs...)
We should get rid of this assumption that we are passing a D-type
anyway. I suggest we define once for all that what we are passing
down the driver is really the overall system state we are getting
to, that is MEM,DISK,KEXEC,SHUTDOWN, eventually STANDBY if we
ever do something like that (useful for handhelds that have a
special idle state and really don't care about scheduling whne
nothing happens for a while).
Ben.
Hi!
> > Thus wrote Bartlomiej Zolnierkiewicz:
> > >
> > > On Thursday 22 of January 2004 17:02, Jeff Garzik wrote:
> > > > 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?
>
> Ideally, it should use the same mecanism as the PM requests...
>
> In fact, the shutdown is just a special case of PM request. I think
> ultimately, we should drop the various "shutdown()" functions in the
> drivers in favor of a "state" selector for PM. That goes along with
> the current problem of "state" in PM beeing completely bogus. The
> constants defined by linux/pm.h are in no way related to what
> the various drivers have come to expect.
>
> enum {
> PM_SUSPEND_ON,
> PM_SUSPEND_STANDBY,
> PM_SUSPEND_MEM,
> PM_SUSPEND_DISK,
> PM_SUSPEND_MAX,
> };
>
> Which basically gives is MEM=2 and DISK=3, while drivers usually
> expect MEM=3 and DISK=4 while nobody really cares about 2 except
> some specific stuffs in the arch code (or radeonfb on pmacs...)
>
> We should get rid of this assumption that we are passing a D-type
> anyway. I suggest we define once for all that what we are passing
> down the driver is really the overall system state we are getting
> to, that is MEM,DISK,KEXEC,SHUTDOWN, eventually STANDBY if we
> ever do something like that (useful for handhelds that have a
> special idle state and really don't care about scheduling whne
> nothing happens for a while).
Agreed. Or at least document that it takes D states
and BUG-ON on invalid values...
Pavel
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms