Re: [PATCH 01/12] scsi_transport_srp: Introduce srp_wait_for_queuecommand()

[Bart Van Assche <bart.vanassche@sandisk.com>]

On 04/30/15 12:33, Sagi Grimberg wrote:
> On 4/30/2015 1:26 PM, Bart Van Assche wrote:
>> On 04/30/15 11:37, Christoph Hellwig wrote:
>>> On Thu, Apr 30, 2015 at 10:56:42AM +0200, Bart Van Assche wrote:
>>>> Introduce the helper function srp_wait_for_queuecommand().
>>>> Move the definition of scsi_request_fn_active(). This patch
>>>> does not change any functionality. A second call to
>>>> scsi_wait_for_queuecommand() will be introduced in the next
>>>> patch.
>>>
>>> Can we take a step back here?
>>>
>>> This isn;t something driver should be doing, so we really should take
>>> care of this in the SCSI midlayer.
>>>
>>> Especially given that we don't maintain request_fn_active for blk-mq.
>>
>> Hello Christoph,
>>
>> How about the following:
>> * Modify scsi_target_block() and scsi_target_unblock(...,
>>     SDEV_TRANSPORT_OFFLINE) such that these functions wait until
>>     active queuecommand() calls have finished.
>
> Won't this make scsi_target_unblock a sleeping function? It might
> not fit all the contexts scsi_target_unblock is called.

The only callers in upstream code of scsi_target_block() and 
scsi_target_unblock() I am aware of are the FC, iSCSI and SRP transport 
layers and the drivers that use these transport layers. As far as I can 
see both functions are always called from thread context and without 
holding any spinlocks. A possible alternative to what I proposed in my 
previous e-mail could be to provide a new function that waits for active 
queuecommand() calls and that has to be called explicitly.

Bart.