Vasu Dev wrote:
> On Tue, 2009-10-20 at 20:54 +0200, Christof Schmitt wrote:
>> On Fri, Oct 16, 2009 at 04:08:24PM -0700, Vasu Dev wrote:
>>> -v3
>>> Moves max_queue_depth initialization after slave_configure is
>>> called from after slave_alloc calling done. Also adjusted
>>> max_queue_depth check to skip ramp up if current queue_depth
>>> is >= max_queue_depth.
>>>
>>> Signed-off-by: Vasu Dev <vasu.dev@xxxxxxxxx>
>> Looks good to me. I ran some tests on s390 and the queue depth counter
>> increased until hitting the defined maximum.
>>
>
> Good.
>
>>> @@ -779,6 +781,36 @@ static struct device_attribute sdev_attr_queue_depth_rw =
>>> sdev_store_queue_depth_rw);
>>>
>>> static ssize_t
>>> +sdev_show_queue_ramp_up_period(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct scsi_device *sdev;
>>> + sdev = to_scsi_device(dev);
>>> + return snprintf(buf, 20, "%lu\n", sdev->queue_ramp_up_period);
>>> +}
>>> +
>>> +static ssize_t
>>> +sdev_store_queue_ramp_up_period(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct scsi_device *sdev = to_scsi_device(dev);
>>> + unsigned long period;
>>> +
>>> + if (strict_strtoul(buf, 10, &period))
>>> + return -EINVAL;
>>> +
>>> + sdev->queue_ramp_up_period = period;
>>> + return period;
>>> +}
>> [...]
>>> + unsigned long queue_ramp_up_period; /* ramp up period in jiffies */
>>> +#define SCSI_DEFAULT_RAMP_UP_PERIOD (120 * HZ)
>> Only a small inconvenience i guess: The sysfs attribute shows the
>> ramp-up-period in jiffies. On my system with HZ==100 the default is
>> 12000 and i was wondering if this would be milliseconds or something
>> related. If HZ changes, the unit of the ramp-up-period also changes.
>>
>> I would prefer having seconds or milliseconds in sysfs and only using
>> jiffies internally.
>>
>
> Added timestamp comparison checks are straightforward without any
> additional conversion on each check since added timestamps and
> queue_ramp_up_period both are jiffies, so works better this way with
> stored queue_ramp_up_period in jiffies.
>
> I do see your point as well but jiffies unit is also common in Linux.
> However if you or some other feels strongly to change this to ms or
> seconds unit then I could change store or show sysfs functions to handle
> this value in ms or second unit while storing it in jiffies in
> queue_ramp_up_period.
sysfs values should be presented in seconds. jiffies is not a user accessible
unit of measure. Esp. for systems with HZ != 100. Milliseconds is too
fine grained (in my opinion).
Mike
>
> Vasu
>
>> Thanks for doing this,
>>
>> Christof
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
