Re: [DO NOT APPLY] sd take advantage of rotation speed

[Matthew Wilcox <matthew@wil.cx>]

On Sun, Jun 22, 2008 at 03:16:49PM +0300, Boaz Harrosh wrote:
> Matthew Wilcox wrote:
> > Use the noop elevator by default for drives that do not spin
> > 
> <snip> 
> > I've brazenly stolen sd_vpd_inquiry from mkp's patch here:
> > 
> > http://marc.info/?l=linux-scsi&m=121264354724277&w=2
> > 
> > No need to have two copies of that ... but this will conflict with his code.
> > 
> <snip>
> > I think there's an opportunity to improve sd_vpd_inquiry() to remove
> > some of the duplicate code between sd_set_elevator() and sd_block_limits,
> > but it's not terribly important.
> > 
> > +static int sd_vpd_inquiry(struct scsi_disk *sdkp, unsigned char *buffer, u8 page, u8 len)
> > +{
> > +	int result;
> > +	unsigned char cmd[16];
> > +	struct scsi_sense_hdr sshdr;
> > +
> > +	memset(cmd, 0, 16);
> > +	cmd[0] = INQUIRY;
> > +	cmd[1] = 1;		/* EVPD */
> > +	cmd[2] = page;		/* VPD page */
> > +	cmd[3] = len;
> > +	
> > +	result = scsi_execute_req(sdkp->device, cmd, DMA_FROM_DEVICE, buffer,
> > +				  len, &sshdr, SD_TIMEOUT, SD_MAX_RETRIES);
> > +
> > +	if (media_not_present(sdkp, &sshdr))
> > +		return -EIO;
> > +
> > +	if (result) {
> > +		sd_printk(KERN_ERR, sdkp, "EVPD Inquiry failed\n");
> > +		return -EIO;
> > +	}
> > +
> > +	if (buffer[1] != page) {
> > +		sd_printk(KERN_ERR, sdkp, "Page code not %2x (%2x)\n", page,
> > +			  buffer[1]);
> > +		return -EIO;
> > +	}
> > +
> > +	return buffer[3];
> > +}
> 
> I was just copy/pasting the same code into my ULD. Perhaps the low level of above
> could be made into a scsi_vpd_inquiry(...). From a quick search it looks like there
> are 3 more places that issue _vpd_inquiry in the tree today.

Yes, there's nothing sd-specific about it.  Let me just make that minor
improvement and I'll send out a proposal for scsi_vpd_inquiry().

> Some of the more interesting device information are in vpd's. we should have a generic
> user-mode way to inquire them. sysfs structure, ioctl, I'm not sure, something good for
> udev. There are tones of such little knobs that could be tuned according to vpd
> information.

We have sg_inq and sg_vpd from Doug's sg3-utils.

> Thanks for doing this, I will investigate if current iscsi-targets will immediately benefit
> from this too. From my testing I found that it is usually better to let the target side
> elevator do the work, and set the Initiator elevator to noop. Do you know what the standard
> say about the default value should be, in the case the page is not present. I would assume 
> it means "No spinning media" too, right?

It means "Not reported".  Which I translate from standard-ese as
"Could be anything".

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."