Re: [PATCH v2 1/4] scsi: introduce sync_before_stop flag

[Aaron Lu <aaron.lu@intel.com>]

On Tue, Sep 18, 2012 at 08:56:55AM +0100, James Bottomley wrote:
> On Tue, 2012-09-18 at 15:00 +0800, Aaron Lu wrote:
> > When scsi device received stop command, it will take care of its
> > internal cache before enters stopped power condition. This command is
> > translated to standby immediate in libata-scsi, but standby doesn't
> > imply flush cache for ATA device, so to issue stop command to ATA
> > device, an additional flush cache has to be issued.
> > 
> > Introduce this flag so that when we are to stop the ATA disk in scsi
> > disk driver, also flush its internal cache.
> > 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > ---
> >  include/scsi/scsi_device.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index 4712aa1..26c3621 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -160,6 +160,7 @@ struct scsi_device {
> >  	unsigned ready_to_power_off:1;	/* Device is ready to be powered off */
> >  	unsigned powered_off:1;	/* Device is powered off */
> >  	unsigned may_power_off:1; /* Power off is allowed by user */
> > +	unsigned sync_before_stop:1; /* Sync cache before stop */
> 
> Why do you need this?
> 
> Surely it's all conditioned on the WCE flag.  If WCE isn't set, the
> cache is write through (or uncached) and there's no need for a sync
> before power down.

The set of this flag doesn't mean we will sync cache for sure.

It's only meaningful when WCE is set, and in that case, it means when we
are to send a stop command to the device, do we need to send an
additional flush cache command first?

In sd_suspend, the cache will be synchronized when:
1 For devices do not support start_stop, always;
2 For devices support start_stop, if it is standard scsi device, never;
  and if it is an ata device(reflected by this newly introduced flag),
  always.

The reason for this is, the scsi's stop command = flush_cache + standby
for ata, so I have to introduce this flag to explicitely do a cache
flush.

Makes sense?

-Aaron