Re: [RFC] [PATCH 1/1] blk request timeout handler patches

[Jens Axboe <jens.axboe@oracle.com>]

On Thu, Oct 11 2007, malahal@us.ibm.com wrote:
> > @@ -3600,24 +3618,132 @@ static struct notifier_block __devinitdata blk_cpu_notifier = {
> >  };
> > 
> >  /**
> > - * blk_complete_request - end I/O on a request
> > - * @req:      the request being processed
> > + * blk_delete_timer - Delete/cancel timer for a given function.
> > + * @req:	request that we are canceling timer for
> >   *
> > - * Description:
> > - *     Ends all I/O on a request. It does not handle partial completions,
> > - *     unless the driver actually implements this in its completion callback
> > - *     through requeueing. Theh actual completion happens out-of-order,
> > - *     through a softirq handler. The user must have registered a completion
> > - *     callback through blk_queue_softirq_done().
> > - **/
> > + * Return value:
> > + *     1 if we were able to detach the timer.  0 if we blew it, and the
> > + *     timer function has already started to run.
> > + */
> > +int blk_delete_timer(struct request *req)
> > +{
> > +	if (!req->q->rq_timed_out_fn)
> > +		return 1;
> > 
> > -void blk_complete_request(struct request *req)
> > +	if (!list_empty(&req->timeout_list)) {
> 
> This could race with timeout handler. In other words, this routine may
> return 1 while blk_rq_timed_out() is running on the request.

My current code is fine, it deletes the request from the list before
calling blk_rq_timed_out().

> > +	ret = q->rq_timed_out_fn(req);
> > +	switch (ret) {
> > +	case BLK_EH_HANDLED:
> > +		__blk_complete_request(req);
> > +		break;
> > +	case BLK_EH_RESET_TIMER:
> > +		blk_add_timer(req);
> 
> You will need to delete the req from the timeout_list before calling
> blk_add_timer again. You may need to remove the req from the list in
> other cases too.

Yep, that is exactly what I did.

-- 
Jens Axboe