Re: [PATCH scsi-misc-2.6 03/05] scsi: make scsi_queue_insert() use blk_requeue_request()

Tejun Heo
2005-04-21T02:29:42Z

James Bottomley wrote:
> On Thu, 2005-04-21 at 09:20 +0900, Tejun Heo wrote:
>
>> Hello, James.
>>
>>James Bottomley wrote:
>>
>>>On Wed, 2005-04-20 at 08:15 +0900, Tejun Heo wrote:
>>>
>>>
>>>>- * Insert this command at the head of the queue for it's device.
>>>>- * It will go before all other commands that are already in the queue.
>>>>- *
>>>>- * NOTE: there is magic here about the way the queue is plugged if
>>>>- * we have no outstanding commands.
>>>>- *
>>>>- * Although this *doesn't* plug the queue, it does call the request
>>>>- * function. The SCSI request function detects the blocked condition
>>>>- * and plugs the queue appropriately.
>>>
>>>
>>>This comment still looks appropriate to me ... why do you want to remove
>>>it?
>>>
>>
>> Well, the thing is that we don't really care what exactly happens to
>>the queue or how the queue is plugged or not. All we need to do are to
>>requeue the request and kick the queue in the ass. Hmmm, maybe I should
>>keep the comment about how the request will be put at the head of the
>>queue, but the second part about plugging doesn't really belong here, I
>>think.
>
>
> Really? We do care greatly. If you requeue with no other outstanding
> commands to the device, the block queue will never restart unless it's
> plugged, and the device will hang. The comment is explaining how this
> happens.
>

Yes, you're right. My point was that that's scsi_run_queue()'s
business. We don't need to comment that deep when we're requeueing a
request. After we put a request on a queue, we kick the queue. It's
the queue running function's responsibility to determine whether to run
the request right away or to defer processing (and thus plug). I wasn't
saying that the eventual plugging isn't necessary, but that the comment
is sort of excessive.

Anyways, if you think the comment is necessary, I don't feel strong
against it. I'll rewrite above comment to fit the new code and repost
this patch soon.

>
>> Yes, that will be more efficient but I don't think it would make
>>any
>>noticeable difference. IMO, universally using scsi_run_queue() to
>>kick
>>scsi request queues is better than mixing blk_run_queue() and
>>scsi_run_queue() for probably unnoticeable optimization. If we start
>>to
>>mix'em, we need to rationalize why specific one is chosen in specific
>>places and that's just unnecessary.
>
>
> Fair enough.

Thanks.

--
tejun