Re: [PATCH v11 2/9] Remove get_device() / put_device() pair from scsi_request_fn()

[Bart Van Assche <bvanassche@acm.org>]

On 06/24/13 15:34, James Bottomley wrote:
> On Mon, 2013-06-24 at 09:13 +0200, Bart Van Assche wrote:
>> On 06/24/13 04:36, James Bottomley wrote:
>>> On Wed, 2013-06-12 at 14:51 +0200, Bart Van Assche wrote:
>>>> Now that all scsi_request_fn() callers hold a reference on the
>>>> SCSI device that function is invoked for
>>>
>>> What makes you think that this is a true statement?  The usual caller is
>>> the block layer, which doesn't really know anything about the
>>> sdev->sdev_gendev.
>>
>> The reasoning behind that comment is as follows:
>> * The block layer guarantees that the reference count of a request
>>     queue is >= 1 as long as a request_fn() call is in progress (see also
>>     blk_cleanup_queue(), the __blk_drain_queue() call in that function
>>     and the loop in __blk_drain_queue() that waits until
>>     request_fn_active == 0).
>> * The SCSI core guarantees that blk_cleanup_queue() is invoked before
>>     the final put on sdev->sdev_gendev.
>
> That's still unclear.
>
> I think a clear explanation is something like:
>
>          scsi_devices may only be removed by by scsi_remove_device()
>          which has a call to blk_cleanup_queue() before the final put of
>          sdev->sdev_gendev.  Since blk_cleanup_queue() waits for the
>          block queue to drain and then tears it down, scsi_request_fn
>          cannot be active once blk_cleanup_queue() returns and hence the
>          get_device/put_device pairs in scsi_request_fn are unnecessary
>          because the final put will always be done by
>          scsi_remove_device().
>
> This, by the way, is an optimisation not a fix, so it shouldn't really
> be part of a series labelled "device removal fixes"

Thanks for the feedback. I will update the patch description and take 
this patch out of the device removal fixes patch series.

Bart.