Re: [PATCH, RFC] scsi: use host wide tags by default

[Jens Axboe <axboe@kernel.dk>]

On 04/17/2015 03:42 PM, James Bottomley wrote:
>> @@ -662,32 +662,14 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
>>    */
>>   int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
>>   {
>> -	unsigned long flags;
>> -
>> -	if (depth <= 0)
>> -		goto out;
>> -
>> -	spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
>> +	if (depth > 0) {
>> +		unsigned long flags;
>>
>> -	/*
>> -	 * Check to see if the queue is managed by the block layer.
>> -	 * If it is, and we fail to adjust the depth, exit.
>> -	 *
>> -	 * Do not resize the tag map if it is a host wide share bqt,
>> -	 * because the size should be the hosts's can_queue. If there
>> -	 * is more IO than the LLD's can_queue (so there are not enuogh
>> -	 * tags) request_fn's host queue ready check will handle it.
>> -	 */
>> -	if (!shost_use_blk_mq(sdev->host) && !sdev->host->bqt) {
>> -		if (blk_queue_tagged(sdev->request_queue) &&
>> -		    blk_queue_resize_tags(sdev->request_queue, depth) != 0)
>> -			goto out_unlock;
>> +		spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
>> +		sdev->queue_depth = depth;
>> +		spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
>
> This lock/unlock is a nasty global sync point which can be eliminated:
> we can rely on the architectural atomicity of 32 bit writes (might need
> to make sdev->queue_depth a u32 because I seem to remember 16 bit writes
> had to be done as two byte stores on some architectures).

It's not in a hot path (by any stretch), so doesn't really matter...

-- 
Jens Axboe