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

2007-10-08 Thread malahal
Jens Axboe [EMAIL PROTECTED] wrote:
 On Thu, Oct 04 2007, [EMAIL PROTECTED] wrote:
  Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
  
  Signed-off-by: Mike Christie [EMAIL PROTECTED]
  Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
  
  
  diff -r 3697367c6e4d block/ll_rw_blk.c
  --- a/block/ll_rw_blk.c Thu Sep 27 00:12:13 2007 -0700
  +++ b/block/ll_rw_blk.c Thu Sep 27 00:13:07 2007 -0700
  @@ -181,6 +181,19 @@ void blk_queue_softirq_done(struct reque
   
   EXPORT_SYMBOL(blk_queue_softirq_done);
   
  +void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout)
  +{
  +   q-rq_timeout = timeout;
  +}
  +EXPORT_SYMBOL_GPL(blk_queue_rq_timeout);
  +
  +void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn)
  +{
  +   q-rq_timed_out_fn = fn;
  +}
  +
  +EXPORT_SYMBOL_GPL(blk_queue_rq_timed_out);
  +
   /**
* blk_queue_make_request - define an alternate make_request function for 
  a device
* @q:  the request queue for the device to be affected
  @@ -243,7 +256,9 @@ static void rq_init(struct request_queue
   {
  INIT_LIST_HEAD(rq-queuelist);
  INIT_LIST_HEAD(rq-donelist);
  -
  +   init_timer(rq-timer);
  +
  +   rq-timeout = 0;
  rq-errors = 0;
  rq-bio = rq-biotail = NULL;
  INIT_HLIST_NODE(rq-hash);
  @@ -2305,6 +2320,7 @@ EXPORT_SYMBOL(blk_start_queueing);
*/
   void blk_requeue_request(struct request_queue *q, struct request *rq)
   {
  +   blk_delete_timer(rq);
  blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
   
  if (blk_rq_tagged(rq))
  @@ -3647,8 +3663,121 @@ static struct notifier_block blk_cpu_not
   };
   
   /**
  + * blk_delete_timer - Delete/cancel timer for a given function.
  + * @req:   request that we are canceling timer for
  + *
  + * 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)
  +{
  +   int rtn;
  +
  +   if (!req-q-rq_timed_out_fn)
  +   return 1;
  +
  +   rtn = del_timer(req-timer);
  +   req-timer.data = (unsigned long)NULL;
  +   req-timer.function = NULL;
  +
  +   return rtn;
  +}
 
 Hmm, that looks fishy. What if the timer _just_ fired, yet it hasn't
 retrieved -data yet?
 

Looks like it is just copied from the SCSI code. More over _run_timers()
copies data and function on to stack variables before calling the timer
function. So, I don't think there is a problem here but assuming that
implementation detail is bad! I would like modify the above function to
reinitialize data and function fields only if del_timer() returns TRUE.
Please let me know if that is OK.

  +
  +EXPORT_SYMBOL_GPL(blk_delete_timer);
  +
  +static void blk_rq_timed_out(struct request *req)
  +{
  +   struct request_queue *q = req-q;
  +
  +   switch (q-rq_timed_out_fn(req)) {
  +   case BLK_EH_HANDLED:
  +   __blk_complete_request(req);
  +   return;
  +   case BLK_EH_RESET_TIMER:
  +   blk_add_timer(req);
  +   return;
  +   case BLK_EH_NOT_HANDLED:
  +   /*
  +* LLD handles this for now but in the future
  +* we can send a request msg to abort the command
  +* and we can move more of the generic scsi eh code to
  +* the blk layer.
  +*/
  +   return;
  +   }
  +}
  +
  +/**
  + * blk_abort_req -- Request request recovery for the specified command
  + * req:pointer to the request of interest
  + *
  + * This function requests that the block layer start recovery for the
  + * request by deleting the timer and calling the q's timeout function.
  + * LLDDs who implement their own error recovery MAY ignore the timeout
  + * event if they generated blk_abort_req.
  + */
  +void blk_abort_req(struct request *req)
  +{
  +if (!blk_delete_timer(req))
  +return;
  +blk_rq_timed_out(req);
  +}
  +
  +EXPORT_SYMBOL_GPL(blk_abort_req);
  +
  +/**
  + * blk_add_timer - Start timeout timer for a single request
  + * @req:   request that is about to start running.
  + *
  + * Notes:
  + *Each request has its own timer, and as it is added to the queue, we
  + *set up the timer.  When the request completes, we cancel the timer.
  + **/
  +void blk_add_timer(struct request *req)
  +{
  +   struct request_queue *q = req-q;
  +
  +   /*
  +* If the clock was already running for this command, then
  +* first delete the timer.  The timer handling code gets rather
  +* confused if we don't do this.
  +*/
  +   if (req-timer.function)
  +   del_timer(req-timer);
  +
  +   req-timer.data = (unsigned long)req;
  +   if (req-timeout)
  +   req-timer.expires = jiffies + req-timeout;
  +   else
  +   req-timer.expires = jiffies + q-rq_timeout;
  +   req-timer.function = (void (*)(unsigned long))blk_rq_timed_out;
 
 Why the cast?
 

Because blk_rq_timed_out() doesn't take unsigned long as its input

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

2007-10-08 Thread Jens Axboe
On Sun, Oct 07 2007, [EMAIL PROTECTED] wrote:
 Jens Axboe [EMAIL PROTECTED] wrote:
  On Thu, Oct 04 2007, [EMAIL PROTECTED] wrote:
   Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
   
   Signed-off-by: Mike Christie [EMAIL PROTECTED]
   Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
   
   
   diff -r 3697367c6e4d block/ll_rw_blk.c
   --- a/block/ll_rw_blk.c   Thu Sep 27 00:12:13 2007 -0700
   +++ b/block/ll_rw_blk.c   Thu Sep 27 00:13:07 2007 -0700
   @@ -181,6 +181,19 @@ void blk_queue_softirq_done(struct reque

EXPORT_SYMBOL(blk_queue_softirq_done);

   +void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout)
   +{
   + q-rq_timeout = timeout;
   +}
   +EXPORT_SYMBOL_GPL(blk_queue_rq_timeout);
   +
   +void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn)
   +{
   + q-rq_timed_out_fn = fn;
   +}
   +
   +EXPORT_SYMBOL_GPL(blk_queue_rq_timed_out);
   +
/**
 * blk_queue_make_request - define an alternate make_request function 
   for a device
 * @q:  the request queue for the device to be affected
   @@ -243,7 +256,9 @@ static void rq_init(struct request_queue
{
 INIT_LIST_HEAD(rq-queuelist);
 INIT_LIST_HEAD(rq-donelist);
   -
   + init_timer(rq-timer);
   +
   + rq-timeout = 0;
 rq-errors = 0;
 rq-bio = rq-biotail = NULL;
 INIT_HLIST_NODE(rq-hash);
   @@ -2305,6 +2320,7 @@ EXPORT_SYMBOL(blk_start_queueing);
 */
void blk_requeue_request(struct request_queue *q, struct request *rq)
{
   + blk_delete_timer(rq);
 blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);

 if (blk_rq_tagged(rq))
   @@ -3647,8 +3663,121 @@ static struct notifier_block blk_cpu_not
};

/**
   + * blk_delete_timer - Delete/cancel timer for a given function.
   + * @req: request that we are canceling timer for
   + *
   + * 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)
   +{
   + int rtn;
   +
   + if (!req-q-rq_timed_out_fn)
   + return 1;
   +
   + rtn = del_timer(req-timer);
   + req-timer.data = (unsigned long)NULL;
   + req-timer.function = NULL;
   +
   + return rtn;
   +}
  
  Hmm, that looks fishy. What if the timer _just_ fired, yet it hasn't
  retrieved -data yet?
  
 
 Looks like it is just copied from the SCSI code. More over _run_timers()
 copies data and function on to stack variables before calling the timer
 function. So, I don't think there is a problem here but assuming that
 implementation detail is bad! I would like modify the above function to
 reinitialize data and function fields only if del_timer() returns TRUE.
 Please let me know if that is OK.

I guess it could be OK, but it then deserves a big fat comment
describing why it is ok.

   +
   +EXPORT_SYMBOL_GPL(blk_delete_timer);
   +
   +static void blk_rq_timed_out(struct request *req)
   +{
   + struct request_queue *q = req-q;
   +
   + switch (q-rq_timed_out_fn(req)) {
   + case BLK_EH_HANDLED:
   + __blk_complete_request(req);
   + return;
   + case BLK_EH_RESET_TIMER:
   + blk_add_timer(req);
   + return;
   + case BLK_EH_NOT_HANDLED:
   + /*
   +  * LLD handles this for now but in the future
   +  * we can send a request msg to abort the command
   +  * and we can move more of the generic scsi eh code to
   +  * the blk layer.
   +  */
   + return;
   + }
   +}
   +
   +/**
   + * blk_abort_req -- Request request recovery for the specified command
   + * req:  pointer to the request of interest
   + *
   + * This function requests that the block layer start recovery for the
   + * request by deleting the timer and calling the q's timeout function.
   + * LLDDs who implement their own error recovery MAY ignore the timeout
   + * event if they generated blk_abort_req.
   + */
   +void blk_abort_req(struct request *req)
   +{
   +if (!blk_delete_timer(req))
   +return;
   +blk_rq_timed_out(req);
   +}
   +
   +EXPORT_SYMBOL_GPL(blk_abort_req);
   +
   +/**
   + * blk_add_timer - Start timeout timer for a single request
   + * @req: request that is about to start running.
   + *
   + * Notes:
   + *Each request has its own timer, and as it is added to the queue, we
   + *set up the timer.  When the request completes, we cancel the timer.
   + **/
   +void blk_add_timer(struct request *req)
   +{
   + struct request_queue *q = req-q;
   +
   + /*
   +  * If the clock was already running for this command, then
   +  * first delete the timer.  The timer handling code gets rather
   +  * confused if we don't do this.
   +  */
   + if (req-timer.function)
   + del_timer(req-timer);
   +
   + req-timer.data = (unsigned long)req;
   + if (req-timeout)
   + req-timer.expires = jiffies + req-timeout;
   + else
   + req-timer.expires = 

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

2007-10-05 Thread Jens Axboe
On Thu, Oct 04 2007, [EMAIL PROTECTED] wrote:
 Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
 
 Signed-off-by: Mike Christie [EMAIL PROTECTED]
 Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
 
 
 diff -r 3697367c6e4d block/ll_rw_blk.c
 --- a/block/ll_rw_blk.c   Thu Sep 27 00:12:13 2007 -0700
 +++ b/block/ll_rw_blk.c   Thu Sep 27 00:13:07 2007 -0700
 @@ -181,6 +181,19 @@ void blk_queue_softirq_done(struct reque
  
  EXPORT_SYMBOL(blk_queue_softirq_done);
  
 +void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout)
 +{
 + q-rq_timeout = timeout;
 +}
 +EXPORT_SYMBOL_GPL(blk_queue_rq_timeout);
 +
 +void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn)
 +{
 + q-rq_timed_out_fn = fn;
 +}
 +
 +EXPORT_SYMBOL_GPL(blk_queue_rq_timed_out);
 +
  /**
   * blk_queue_make_request - define an alternate make_request function for a 
 device
   * @q:  the request queue for the device to be affected
 @@ -243,7 +256,9 @@ static void rq_init(struct request_queue
  {
   INIT_LIST_HEAD(rq-queuelist);
   INIT_LIST_HEAD(rq-donelist);
 -
 + init_timer(rq-timer);
 +
 + rq-timeout = 0;
   rq-errors = 0;
   rq-bio = rq-biotail = NULL;
   INIT_HLIST_NODE(rq-hash);
 @@ -2305,6 +2320,7 @@ EXPORT_SYMBOL(blk_start_queueing);
   */
  void blk_requeue_request(struct request_queue *q, struct request *rq)
  {
 + blk_delete_timer(rq);
   blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
  
   if (blk_rq_tagged(rq))
 @@ -3647,8 +3663,121 @@ static struct notifier_block blk_cpu_not
  };
  
  /**
 + * blk_delete_timer - Delete/cancel timer for a given function.
 + * @req: request that we are canceling timer for
 + *
 + * 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)
 +{
 + int rtn;
 +
 + if (!req-q-rq_timed_out_fn)
 + return 1;
 +
 + rtn = del_timer(req-timer);
 + req-timer.data = (unsigned long)NULL;
 + req-timer.function = NULL;
 +
 + return rtn;
 +}

Hmm, that looks fishy. What if the timer _just_ fired, yet it hasn't
retrieved -data yet?

 +
 +EXPORT_SYMBOL_GPL(blk_delete_timer);
 +
 +static void blk_rq_timed_out(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + switch (q-rq_timed_out_fn(req)) {
 + case BLK_EH_HANDLED:
 + __blk_complete_request(req);
 + return;
 + case BLK_EH_RESET_TIMER:
 + blk_add_timer(req);
 + return;
 + case BLK_EH_NOT_HANDLED:
 + /*
 +  * LLD handles this for now but in the future
 +  * we can send a request msg to abort the command
 +  * and we can move more of the generic scsi eh code to
 +  * the blk layer.
 +  */
 + return;
 + }
 +}
 +
 +/**
 + * blk_abort_req -- Request request recovery for the specified command
 + * req:  pointer to the request of interest
 + *
 + * This function requests that the block layer start recovery for the
 + * request by deleting the timer and calling the q's timeout function.
 + * LLDDs who implement their own error recovery MAY ignore the timeout
 + * event if they generated blk_abort_req.
 + */
 +void blk_abort_req(struct request *req)
 +{
 +if (!blk_delete_timer(req))
 +return;
 +blk_rq_timed_out(req);
 +}
 +
 +EXPORT_SYMBOL_GPL(blk_abort_req);
 +
 +/**
 + * blk_add_timer - Start timeout timer for a single request
 + * @req: request that is about to start running.
 + *
 + * Notes:
 + *Each request has its own timer, and as it is added to the queue, we
 + *set up the timer.  When the request completes, we cancel the timer.
 + **/
 +void blk_add_timer(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + /*
 +  * If the clock was already running for this command, then
 +  * first delete the timer.  The timer handling code gets rather
 +  * confused if we don't do this.
 +  */
 + if (req-timer.function)
 + del_timer(req-timer);
 +
 + req-timer.data = (unsigned long)req;
 + if (req-timeout)
 + req-timer.expires = jiffies + req-timeout;
 + else
 + req-timer.expires = jiffies + q-rq_timeout;
 + req-timer.function = (void (*)(unsigned long))blk_rq_timed_out;

Why the cast?

 +add_timer(req-timer);

You have space vs tab issues.

 +}

That's an ugly interface. I'd say it's a bug to call blk_add_timer()
with it already pending, so fix the API.

Apart from that, this has been a TODO item for me for some time. So
thanks for refreshing these, lets see if we can get them beat into
submission and agree on how it should work so we can take this forward.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info 

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

2007-10-05 Thread Jens Axboe

(resend, with proper CC - stupid mutt)

On Thu, Oct 04 2007, [EMAIL PROTECTED] wrote:
 Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
 
 Signed-off-by: Mike Christie [EMAIL PROTECTED]
 Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
 
 
 diff -r 3697367c6e4d block/ll_rw_blk.c
 --- a/block/ll_rw_blk.c   Thu Sep 27 00:12:13 2007 -0700
 +++ b/block/ll_rw_blk.c   Thu Sep 27 00:13:07 2007 -0700
 @@ -181,6 +181,19 @@ void blk_queue_softirq_done(struct reque
  
  EXPORT_SYMBOL(blk_queue_softirq_done);
  
 +void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout)
 +{
 + q-rq_timeout = timeout;
 +}
 +EXPORT_SYMBOL_GPL(blk_queue_rq_timeout);
 +
 +void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn)
 +{
 + q-rq_timed_out_fn = fn;
 +}
 +
 +EXPORT_SYMBOL_GPL(blk_queue_rq_timed_out);
 +
  /**
   * blk_queue_make_request - define an alternate make_request function for a 
 device
   * @q:  the request queue for the device to be affected
 @@ -243,7 +256,9 @@ static void rq_init(struct request_queue
  {
   INIT_LIST_HEAD(rq-queuelist);
   INIT_LIST_HEAD(rq-donelist);
 -
 + init_timer(rq-timer);
 +
 + rq-timeout = 0;
   rq-errors = 0;
   rq-bio = rq-biotail = NULL;
   INIT_HLIST_NODE(rq-hash);
 @@ -2305,6 +2320,7 @@ EXPORT_SYMBOL(blk_start_queueing);
   */
  void blk_requeue_request(struct request_queue *q, struct request *rq)
  {
 + blk_delete_timer(rq);
   blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
  
   if (blk_rq_tagged(rq))
 @@ -3647,8 +3663,121 @@ static struct notifier_block blk_cpu_not
  };
  
  /**
 + * blk_delete_timer - Delete/cancel timer for a given function.
 + * @req: request that we are canceling timer for
 + *
 + * 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)
 +{
 + int rtn;
 +
 + if (!req-q-rq_timed_out_fn)
 + return 1;
 +
 + rtn = del_timer(req-timer);
 + req-timer.data = (unsigned long)NULL;
 + req-timer.function = NULL;
 +
 + return rtn;
 +}

Hmm, that looks fishy. What if the timer _just_ fired, yet it hasn't
retrieved -data yet?

 +
 +EXPORT_SYMBOL_GPL(blk_delete_timer);
 +
 +static void blk_rq_timed_out(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + switch (q-rq_timed_out_fn(req)) {
 + case BLK_EH_HANDLED:
 + __blk_complete_request(req);
 + return;
 + case BLK_EH_RESET_TIMER:
 + blk_add_timer(req);
 + return;
 + case BLK_EH_NOT_HANDLED:
 + /*
 +  * LLD handles this for now but in the future
 +  * we can send a request msg to abort the command
 +  * and we can move more of the generic scsi eh code to
 +  * the blk layer.
 +  */
 + return;
 + }
 +}
 +
 +/**
 + * blk_abort_req -- Request request recovery for the specified command
 + * req:  pointer to the request of interest
 + *
 + * This function requests that the block layer start recovery for the
 + * request by deleting the timer and calling the q's timeout function.
 + * LLDDs who implement their own error recovery MAY ignore the timeout
 + * event if they generated blk_abort_req.
 + */
 +void blk_abort_req(struct request *req)
 +{
 +if (!blk_delete_timer(req))
 +return;
 +blk_rq_timed_out(req);
 +}
 +
 +EXPORT_SYMBOL_GPL(blk_abort_req);
 +
 +/**
 + * blk_add_timer - Start timeout timer for a single request
 + * @req: request that is about to start running.
 + *
 + * Notes:
 + *Each request has its own timer, and as it is added to the queue, we
 + *set up the timer.  When the request completes, we cancel the timer.
 + **/
 +void blk_add_timer(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + /*
 +  * If the clock was already running for this command, then
 +  * first delete the timer.  The timer handling code gets rather
 +  * confused if we don't do this.
 +  */
 + if (req-timer.function)
 + del_timer(req-timer);
 +
 + req-timer.data = (unsigned long)req;
 + if (req-timeout)
 + req-timer.expires = jiffies + req-timeout;
 + else
 + req-timer.expires = jiffies + q-rq_timeout;
 + req-timer.function = (void (*)(unsigned long))blk_rq_timed_out;

Why the cast?

 +add_timer(req-timer);

You have space vs tab issues.

 +}

That's an ugly interface. I'd say it's a bug to call blk_add_timer()
with it already pending, so fix the API.

Apart from that, this has been a TODO item for me for some time. So
thanks for refreshing these, lets see if we can get them beat into
submission and agree on how it should work so we can take this forward.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message 

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

2007-10-04 Thread Randy Dunlap
On Thu, 4 Oct 2007 11:17:50 -0700 [EMAIL PROTECTED] wrote:

 Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
 
 Signed-off-by: Mike Christie [EMAIL PROTECTED]
 Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
 
 
 diff -r 3697367c6e4d block/ll_rw_blk.c
 --- a/block/ll_rw_blk.c   Thu Sep 27 00:12:13 2007 -0700
 +++ b/block/ll_rw_blk.c   Thu Sep 27 00:13:07 2007 -0700
 @@ -181,6 +181,19 @@ void blk_queue_softirq_done(struct reque
  
  EXPORT_SYMBOL(blk_queue_softirq_done);
  
 +void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout)
 +{
 + q-rq_timeout = timeout;
 +}
 +EXPORT_SYMBOL_GPL(blk_queue_rq_timeout);
 +
 +void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn)
 +{
 + q-rq_timed_out_fn = fn;
 +}
 +

Drop that blank line.

 +EXPORT_SYMBOL_GPL(blk_queue_rq_timed_out);
 +
  /**
   * blk_queue_make_request - define an alternate make_request function for a 
 device
   * @q:  the request queue for the device to be affected
 @@ -3647,8 +3663,121 @@ static struct notifier_block blk_cpu_not
  };
  
  /**
 + * blk_delete_timer - Delete/cancel timer for a given function.
 + * @req: request that we are canceling timer for
 + *
 + * 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)
 +{
 + int rtn;
 +
 + if (!req-q-rq_timed_out_fn)
 + return 1;
 +
 + rtn = del_timer(req-timer);
 + req-timer.data = (unsigned long)NULL;
 + req-timer.function = NULL;
 +
 + return rtn;
 +}
 +

ditto.

 +EXPORT_SYMBOL_GPL(blk_delete_timer);
 +
 +static void blk_rq_timed_out(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + switch (q-rq_timed_out_fn(req)) {
 + case BLK_EH_HANDLED:
 + __blk_complete_request(req);
 + return;
 + case BLK_EH_RESET_TIMER:
 + blk_add_timer(req);
 + return;
 + case BLK_EH_NOT_HANDLED:
 + /*
 +  * LLD handles this for now but in the future
 +  * we can send a request msg to abort the command
 +  * and we can move more of the generic scsi eh code to
 +  * the blk layer.
 +  */
 + return;
 + }
 +}
 +
 +/**
 + * blk_abort_req -- Request request recovery for the specified command
 + * req:  pointer to the request of interest
 + *

s/req:/@req:/

 + * This function requests that the block layer start recovery for the
 + * request by deleting the timer and calling the q's timeout function.
 + * LLDDs who implement their own error recovery MAY ignore the timeout
 + * event if they generated blk_abort_req.
 + */
 +void blk_abort_req(struct request *req)
 +{
 +if (!blk_delete_timer(req))
 +return;
 +blk_rq_timed_out(req);
 +}
 +

drop blank line between closing } of function and the following
EXPORT_SYMBOL...

 +EXPORT_SYMBOL_GPL(blk_abort_req);
 +
 +/**
 + * blk_add_timer - Start timeout timer for a single request
 + * @req: request that is about to start running.
 + *
 + * Notes:
 + *Each request has its own timer, and as it is added to the queue, we
 + *set up the timer.  When the request completes, we cancel the timer.
 + **/

*/

 +void blk_add_timer(struct request *req)
 +{
 + struct request_queue *q = req-q;
 +
 + /*
 +  * If the clock was already running for this command, then
 +  * first delete the timer.  The timer handling code gets rather
 +  * confused if we don't do this.
 +  */
 + if (req-timer.function)
 + del_timer(req-timer);
 +
 + req-timer.data = (unsigned long)req;
 + if (req-timeout)
 + req-timer.expires = jiffies + req-timeout;
 + else
 + req-timer.expires = jiffies + q-rq_timeout;
 + req-timer.function = (void (*)(unsigned long))blk_rq_timed_out;
 +add_timer(req-timer);
 +}
 +

drop blank line.  Please just check the rest of them.

 +EXPORT_SYMBOL_GPL(blk_add_timer);
 +
 +/**
   * blk_complete_request - end I/O on a request
 - * @req:  the request being processed
 + * @req: the request being processed
   *
   * Description:
   * Ends all I/O on a request. It does not handle partial completions,
 @@ -3657,25 +3786,24 @@ static struct notifier_block blk_cpu_not
   * through a softirq handler. The user must have registered a completion
   * callback through blk_queue_softirq_done().
   **/

*/

 -
  void blk_complete_request(struct request *req)
  {


---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-04 Thread Salyzyn, Mark
ACK for the trivial portion surrounding aacraid and ips!

Sincerely -- Mark Salyzyn
 
 -Original Message-
 From: [EMAIL PROTECTED] 
 [mailto:[EMAIL PROTECTED] On Behalf Of 
 [EMAIL PROTECTED]
 Sent: Thursday, October 04, 2007 2:18 PM
 To: linux-scsi@vger.kernel.org; [EMAIL PROTECTED]
 Subject: [RFC] [PATCH 1/2] blk request timeout handler patches
 
 Mike Christie's patches refreshed to 2.6.23-rc8-mm1.
 
 Signed-off-by: Mike Christie [EMAIL PROTECTED]
 Signed-off-by: Malahal Naineni [EMAIL PROTECTED]
. . .
 diff -r 3697367c6e4d drivers/scsi/aacraid/aachba.c
 --- a/drivers/scsi/aacraid/aachba.c   Thu Sep 27 00:12:13 2007 -0700
 +++ b/drivers/scsi/aacraid/aachba.c   Thu Sep 27 00:13:07 2007 -0700
 @@ -1125,7 +1125,7 @@ static struct aac_srb * aac_scsi_common(
   srbcmd-id   = cpu_to_le32(scmd_id(cmd));
   srbcmd-lun  = cpu_to_le32(cmd-device-lun);
   srbcmd-flags= cpu_to_le32(flag);
 - timeout = cmd-timeout_per_command/HZ;
 + timeout = cmd-request-timeout/HZ;
   if (timeout == 0)
   timeout = 1;
   srbcmd-timeout  = cpu_to_le32(timeout);  // timeout in seconds
. . .
 diff -r 3697367c6e4d drivers/scsi/ips.c
 --- a/drivers/scsi/ips.c  Thu Sep 27 00:12:13 2007 -0700
 +++ b/drivers/scsi/ips.c  Thu Sep 27 00:13:07 2007 -0700
 @@ -3862,7 +3862,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * 
   scb-cmd.dcdb.segment_4G = 0;
   scb-cmd.dcdb.enhanced_sg = 0;
  
 - TimeOut = scb-scsi_cmd-timeout_per_command;
 + TimeOut = scb-scsi_cmd-request-timeout;
  
   if (ha-subsys-param[4]  0x0010) {
 /* If NEW Tape DCDB is Supported */
   if (!scb-sg_len) {
. . .
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html