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

[Boaz Harrosh <bharrosh@panasas.com>]

Matthew Wilcox wrote:
> 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".
> 

OK Thanks. I'll make sure all iscsi targets I care about return this page.
Am anticipating this patchset to be submitted, thanks again.

Boaz