Re: ata_ram driver

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2008-03-07 at 08:55 +0900, Tejun Heo wrote:
> James Bottomley wrote:
> >> It's because of the sequence of events.  Currently, driver unload
> >> sequence is the same as when the device is hot unplugged.  libata
> >> detects that the device is gone and disables it and report it to SCSI
> >> layer.  SCSI layer takes over and tries to kill the SCSI device and tell
> >> sd to shutdown and sd issues START_STOP to shutdown which fails w/
> >> DID_BAD_TARGET because the matching ATA device is already gone.  I've
> >> left it that way because I'm not sure whether spinning down the drive on
> >> driver unload is the correct thing to do.  The message is annoying tho.
> > 
> > Um, it's not supposed to happen that way.  Your signal that a disk is
> > gone is slave destroy ... and we don't call that until after the target
> > has been processed.  Devices are supposed to stay online (if possible)
> > from slave alloc to slave destroy.
> 
> Currently, it's like the following.
> 
> * Explicit unplug request via sysfs or whatever: ATA device stays online
> till slave_destroy finishes.

That's correct

> * Hot unplugging: Nothing much libata can do.  ATA device is yanked out
> by the user.

Yes, us too ... you just have to error all commands when the device
vanishes.

> * Driver unload: Dealt the same way as hot unplugging.

This is the problem case: driver unloading should have a
scsi_remove_host() in its path.  This is the trigger that sends out the
flushes/stops and calls slave_destroy.  scsi_remove_host() doesn't
actually return until all the destroys are completed, so it makes module
unloading wait until everything is properly shut down.

> Making driver unload like explicit unplug request is possible but it
> will mean that drives will be spun down on driver unload, which can be
> annoying to developers. 

You have a sysfs flag to prevent that, don't you?

>  In addition, the code path is shared with
> controller hot unplug in which case it's probably best not to issue any
> new command.  So, I've been reluctant to make the change.  If the change
> is required, I think it can be done by adding a few lines at the top of
> ata_port_detach().  Jeff, what do you think?

James