Re: Fix 2.5.34+swsusp data corruption on IDE

[Pavel Machek <pavel@ucw.cz>]

Hi!

> I spoke with Jens, and he wants us to hold off until we can settle the
> current issues.  I have one concern about the model, and maybe you can
> explain away the concern.

Jens, where is the problem? This should have absolutely zero impact on
"interesting" code, making only changes to initialization and
suspend/resume...

> Why are we not blocking read/write requests in the mainloop regardless?
> If the request gets to the subdriver, ide-disk, has it not gotten to far
> down the pipes?

All processes capable of generating requests are safely stopped, so no
request should get down to drivers. That's why I simply BUG_ON(), not
block or anything more sophiscated. It should never ever happen.

> Specifically ls120's and zips.
> 
> I understand you are address disk but suspend is more than disk in the
> power management picture.  Can you walk me through your process of sole
> concern with platter media?  Remember microdrvies are platters too, as are
> flash drives, and memory drives.

High levels are stopped, so there are no new requests coming.

What is needed in idedisk_suspend is to make sure that no requests are
"in flight". DMA scribbling over random memory is not good thing.
idedisk_suspend then sends drive to standby to make sure writeback
caches are flushed.

as ls120s and zips... If they are DMA capable, they badly need
support. If not, they should not kill rest of the system during
suspend-to-disk, but still support would be nice.

> > +static int idedisk_suspend(struct device *dev, u32 state, u32 level)
> > +{
> > +	ide_drive_t *drive = dev->driver_data;
> > +
> > +	/* I hope that every freeze operations from the upper levels have
> > +	 * already been done...
> > +	 */
> > +
> > +	BUG_ON(in_interrupt());
> > +
> > +	if (level != SUSPEND_SAVE_STATE)
> > +		return 0;
> > +
> > +	/* wait until all commands are finished */
> > +	/* FIXME: waiting for spinlocks should be done instead. */
> > +	while (HWGROUP(drive)->handler)
> > +		yield();
> > +
> > +	/* set the drive to standby */
> > +	printk(KERN_INFO "suspending: %s ", drive->name);
> > +	if (drive->driver) {
> > +		if (drive->driver->standby)
> > +			drive->driver->standby(drive);
> > +	}
> > +	drive->blocked = 1;
> > +
> > +	return 0;

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.