On 06/06/12 13:29, Mike Christie wrote:
> On 06/06/2012 07:17 AM, Bart Van Assche wrote:
>> On 06/05/12 21:36, Mike Christie wrote:
>>
>>> On 06/05/2012 12:14 PM, Bart Van Assche wrote:
>>>> Avoid that the code for requeueing SCSI requests triggers a
>>>> crash by making sure that that code isn't scheduled anymore
>>>> after a device has been removed.
>>>>
>>>> Also, source code inspection of __scsi_remove_device() revealed
>>>> a race condition in this function: no new SCSI requests must be
>>>> accepted for a SCSI device after device removal started.
>>>>
>>>> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
>>>> Cc: Mike Christie <michaelc@xxxxxxxxxxx>
>>>> Cc: James Bottomley <JBottomley@xxxxxxxxxxxxx>
>>>> Cc: Jens Axboe <axboe@xxxxxxxxx>
>>>> Cc: Joe Lawrence <jdl1291@xxxxxxxxx>
>>>> Cc: Jun'ichi Nomura <j-nomura@xxxxxxxxxxxxx>
>>>> Cc: <stable@xxxxxxxxxx>
>>>> ---
>>>> drivers/scsi/scsi_lib.c | 7 ++++---
>>>> drivers/scsi/scsi_sysfs.c | 11 +++++++++--
>>>> 2 files changed, 13 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>>>> index 082c1e5..b722a8b 100644
>>>> --- a/drivers/scsi/scsi_lib.c
>>>> +++ b/drivers/scsi/scsi_lib.c
>>>> @@ -158,10 +158,11 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
>>>> * that are already in the queue.
>>>> */
>>>> spin_lock_irqsave(q->queue_lock, flags);
>>>> - blk_requeue_request(q, cmd->request);
>>>> + if (!blk_queue_dead(q)) {
>>>> + blk_requeue_request(q, cmd->request);
>>>> + kblockd_schedule_work(q, &device->requeue_work);
>>>> + }
>>>
>>> If we do not requeue what eventually frees the request?
>>
>> As far as I can see any request passed to __scsi_queue_insert() has
>> already been started. So if it isn't requeued it's timer remains active
>> and hence will fire eventually.
>
> This is true in the scsi_dispatch_cmd path, but not others.
>
> If we were requeueing from the error handler scsi_eh_flush_done_q then
> the timer would have been stopped because that is how we got into the eh.
Given the above I propose to add a __blk_end_request_all(req, -ENXIO)
call if the queue is dead.
Bart.
--
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
