On Fri, Jan 28 2005, James Bottomley wrote:
> On Fri, 2005-01-28 at 10:38 +0100, Jens Axboe wrote:
> > +/*
> > + * snoop succesfull completion of mode select commands that update the
> > + * write back cache state
> > + */
> > +#define MS_CACHE_PAGE 0x08
> > +static void sd_snoop_cmd(struct scsi_cmnd *cmd)
> > +{
> > + struct scsi_disk *sdpk;
> > + char *page;
> > +
> > + if (cmd->result)
> > + return;
> > +
> > + switch (cmd->cmnd[0]) {
> > + case MODE_SELECT:
> > + case MODE_SELECT_10:
> > + page = cmd->request_buffer;
> > + if (!page)
> > + break;
> > + if ((page[0] & 0x3f) != MS_CACHE_PAGE)
> > + break;
> > + sdpk = dev_get_drvdata(&cmd->device->sdev_gendev);
> > + sdpk->WCE = (page[2] & 0x04) != 0;
> > + break;
> > + }
> > +}
> > +
> > /**
> > * sd_rw_intr - bottom half handler: called when the lower level
> > * driver has completed (successfully or otherwise) a scsi command.
> > @@ -773,6 +831,9 @@ static void sd_rw_intr(struct scsi_cmnd
> > SCpnt->sense_buffer[13]));
> > }
> > #endif
> > +
> > + sd_snoop_cmd(SCpnt);
> > +
>
> Good grief no!
>
> If you're going to try something like this, it needs to be a separate
> patch over the scsi-list for one thing. And to save time:
>
> 1) The patch is actually wrong. There's more than one caching mode page
> and not all of them affect current behaviour.
It also gets the offset wrong :)
> 2) We have a current interface to update the WCE bit: You twiddle all
> the disc parameters and then trigger a device rescan via sysfs (I'll
> check that this updates the cache bits, I think it does, but if it
> doesn't I'll make it).
> 3) If we think this is a quantity the users would like to see and alter,
> then reading and setting it should be exported via sysfs.
> 4) Snooping SCSI commands is really bad ... it can get you into all
> sorts of trouble which is why we prefer asking the device what state
> it's in to trying to guess ourselves.
I would _much_ prefer some sort of easily tweakable way to change the
write back mode, if this is something we want to support. As I wrote in
the original reply, I hate the concept of command snooping.
The barrier stuff works fine as-is, I'll just rely on scsi getting the
WCE updating correct.
--
Jens Axboe
-
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/
