Re: [PATCH 1/3] Remove two cancel_delayed_work() calls from the error handler

[Hannes Reinecke <hare@suse.de>]

(Resending; mailer rejected it ...)

On 05/27/2014 08:08 AM, Bart Van Assche wrote:
> On 05/27/14 07:40, Hannes Reinecke wrote:
>> On 05/26/2014 05:14 PM, Bart Van Assche wrote:
>>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>>> index f17aa7a..5232583 100644
>>> --- a/drivers/scsi/scsi_error.c
>>> +++ b/drivers/scsi/scsi_error.c
>>> @@ -193,7 +193,7 @@ scsi_abort_command(struct scsi_cmnd *scmd)
>>>            SCSI_LOG_ERROR_RECOVERY(3,
>>>                scmd_printk(KERN_INFO, scmd,
>>>                        "scmd %p previous abort failed\n", scmd));
>>> -        cancel_delayed_work(&scmd->abort_work);
>>> +        WARN_ON_ONCE(delayed_work_pending(&scmd->abort_work));
>>>            return FAILED;
>>>        }
>>>
>>>
>> The first bit is okay, the second isn't.
>>
>> The second bit is for these cases where the abort got scheduled (in
>> scsi_abort_command()), but the workqueue didn't get executed by the time
>> the next timeout occured.
>> I know, highly unlikely, but there is no safeguarding that it _cannot_
>> happen.
>> So the second cancel_delayed_work() has to stay.
>
> But how could that next timeout occur while abort_work is still pending
> ? The block layer removes a request from the timeout list before
> invoking the timeout handler (see also blk_rq_check_expired()). This
> means that no block layer timers are active after abort_work has been
> scheduled and before scmd_eh_abort_handler() is called. This also means
> that a second timeout can only occur after a SCSI command has been
> reinserted to a SCSI device queue. And such a reinsertion can only occur
> after scmd_eh_abort_handler() has started. The pending bit is cleared
> from a work struct before the associated handler is invoked. This is why
> I think the above cancel_delayed_work() statement is not necessary. Or
> did I perhaps overlook something ?
>
Hmm. Okay, convinced.

Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html