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

[malahal@us.ibm.com]

Jens Axboe [jens.axboe@oracle.com] wrote:
> Current code is below, btw. Not a lot of changes, iirc it's just the
> delete-always, a missing export, delete timer on empty list.
> 
> +void blk_add_timer(struct request *req)
> +{
> +	struct request_queue *q = req->q;
> +	unsigned long expiry;
> +
> +	BUG_ON(!list_empty(&req->timeout_list));
> +
> +	if (req->timeout)
> +		req->timeout += jiffies;
> +	else
> +		req->timeout = jiffies + q->rq_timeout;
> +

The meaning of req->timeout is changed here. It was a timeout value, now
it became an expiry time. If we happen to requeue the request, its
timeout would be incorrect. Found using scsi_debug! Maybe, make it a
bitfield to avoid another field...

> +	list_add_tail(&req->timeout_list, &q->timeout_list);
> +
> +	/*
> +	 * If the timer isn't already pending or this timeout is earlier
> +	 * than an existing one, modify the timer. Round to next nearest
> +	 * second.
> +	 */
> +	expiry = round_jiffies(req->timeout);
> +	if (!timer_pending(&q->timeout) ||
> +	    time_before(expiry, q->timeout.expires))
> +		mod_timer(&q->timeout, expiry);

How about changing q->timeout to q->timer?

> diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
> index 32982eb..22a9013 100644
> --- a/drivers/scsi/gdth_proc.c
> +++ b/drivers/scsi/gdth_proc.c
> @@ -846,19 +846,19 @@ static int gdth_update_timeout(int hanum, Scsi_Cmnd *scp, int timeout)
>  {
>      int oldto;
> 
> -    oldto = scp->timeout_per_command;
> -    scp->timeout_per_command = timeout;
> +    oldto = scp->request->timeout;
> +    scp->request->timeout = timeout;
> 
>      if (timeout == 0) {
> -        del_timer(&scp->eh_timeout);
> -        scp->eh_timeout.data = (unsigned long) NULL;
> -        scp->eh_timeout.expires = 0;
> +        del_timer(&scp->request->timer);
> +        scp->request->timer.data = (unsigned long) NULL;
> +        scp->request->timer.expires = 0;

request doesn't have "timer" field, code compilation fails for this
file.