Re: [PATCH] blk request timeout minor fixes...

2007-12-05 Thread malahal
Jens Axboe [EMAIL PROTECTED] wrote:
  This one fixes the 4-byte hole. Thank you very much.
 
 That's all fine, but now we have two fields doing essentially the same
 thing. Care to cleanup the -timeout usage so we can get by with using
 just that field?

I added another field because I couldn't find a way to use just one
field without breaking something! I will gladly clean it up if you
find a way to fix it.

Thanks, Malahal.
-
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: [PATCH] blk request timeout minor fixes...

2007-12-05 Thread Jens Axboe
On Wed, Dec 05 2007, [EMAIL PROTECTED] wrote:
 Jens Axboe [EMAIL PROTECTED] wrote:
   This one fixes the 4-byte hole. Thank you very much.
  
  That's all fine, but now we have two fields doing essentially the same
  thing. Care to cleanup the -timeout usage so we can get by with using
  just that field?
 
 I added another field because I couldn't find a way to use just one
 field without breaking something! I will gladly clean it up if you
 find a way to fix it.

-timeout isn't used very much, so should not be too much work to
console the two.

If you see any specific obstacles, do list them!

-- 
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 at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] blk request timeout minor fixes...

2007-12-05 Thread malahal
Jens Axboe [EMAIL PROTECTED] wrote:
 -timeout isn't used very much, so should not be too much work to
 console the two.
 
 If you see any specific obstacles, do list them!

I believe it is used in blk_add_timer() which may be called:
1) First time sending the command to LLDs
   No problem. It would be set to correct value at that time.
2) called from blk_rq_timed_out() in response to BLK_EH_RESET_TIMER
   Without the actual timeout value, there is no way to res-set timer
   correctly.
3) Retry
   Same #2.

The other way is to remove the deadline field. That poses timing out
issues.

Thanks, Malahal.
-
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: [PATCH] blk request timeout minor fixes...

2007-12-04 Thread Jens Axboe
On Mon, Dec 03 2007, [EMAIL PROTECTED] wrote:
 Matthew Wilcox [EMAIL PROTECTED] wrote:
  On Sun, Dec 02, 2007 at 05:53:30PM -0800, [EMAIL PROTECTED] wrote:
  Can I suggest running 'pahole' over this when compiled on 64-bit?
  You've just introduced a 4-byte hole.
 
 This one fixes the 4-byte hole. Thank you very much.

That's all fine, but now we have two fields doing essentially the same
thing. Care to cleanup the -timeout usage so we can get by with using
just that field?

-- 
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 at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] blk request timeout minor fixes...

2007-12-03 Thread malahal
Matthew Wilcox [EMAIL PROTECTED] wrote:
 On Sun, Dec 02, 2007 at 05:53:30PM -0800, [EMAIL PROTECTED] wrote:
 Can I suggest running 'pahole' over this when compiled on 64-bit?
 You've just introduced a 4-byte hole.

This one fixes the 4-byte hole. Thank you very much.


diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c
index 79ed268..5e95fa8 100644
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -259,6 +259,7 @@ static void rq_init(struct request_queue *q, struct request 
*rq)
INIT_LIST_HEAD(rq-timeout_list);
 
rq-timeout = 0;
+   rq-deadline = 0;
rq-errors = 0;
rq-bio = rq-biotail = NULL;
INIT_HLIST_NODE(rq-hash);
@@ -3707,21 +3708,24 @@ static void blk_rq_timed_out(struct request *req)
 static void blk_rq_timed_out_timer(unsigned long data)
 {
struct request_queue *q = (struct request_queue *) data;
-   unsigned long flags, next = 0;
+   unsigned long flags, uninitialized_var(next), next_set = 0;
struct request *rq, *tmp;
 
spin_lock_irqsave(q-queue_lock, flags);
 
list_for_each_entry_safe(rq, tmp, q-timeout_list, timeout_list) {
-   if (!next || time_before(next, rq-timeout))
-   next = rq-timeout;
-   if (time_after_eq(jiffies, rq-timeout)) {
+   if (time_after_eq(jiffies, rq-deadline)) {
list_del_init(rq-timeout_list);
blk_rq_timed_out(rq);
}
+   if (!next_set) {
+   next = rq-deadline;
+   next_set = 1;
+   } else if (time_after(next, rq-deadline))
+   next = rq-deadline;
}
 
-   if (next)
+   if (next_set)
mod_timer(q-timeout, round_jiffies(next));
 
spin_unlock_irqrestore(q-queue_lock, flags);
@@ -3760,9 +3764,9 @@ void blk_add_timer(struct request *req)
BUG_ON(!list_empty(req-timeout_list));
 
if (req-timeout)
-   req-timeout += jiffies;
+   req-deadline = jiffies + req-timeout;
else
-   req-timeout = jiffies + q-rq_timeout;
+   req-deadline = jiffies + q-rq_timeout;
 
list_add_tail(req-timeout_list, q-timeout_list);
 
@@ -3771,7 +3775,7 @@ void blk_add_timer(struct request *req)
 * than an existing one, modify the timer. Round to next nearest
 * second.
 */
-   expiry = round_jiffies(req-timeout);
+   expiry = round_jiffies(req-deadline);
if (!timer_pending(q-timeout) ||
time_before(expiry, q-timeout.expires))
mod_timer(q-timeout, expiry);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 917fe86..834d097 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -295,8 +295,9 @@ struct request {
void *data;
void *sense;
 
-   unsigned long timeout;
+   unsigned long deadline;
struct list_head timeout_list;
+   unsigned int timeout;
int retries;
 
/*
-
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: [PATCH] blk request timeout minor fixes...

2007-12-02 Thread Matthew Wilcox
On Sun, Dec 02, 2007 at 05:53:30PM -0800, [EMAIL PROTECTED] wrote:
 diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
 index 917fe86..e495d61 100644
 --- a/include/linux/blkdev.h
 +++ b/include/linux/blkdev.h
 @@ -295,7 +295,8 @@ struct request {
   void *data;
   void *sense;
  
 - unsigned long timeout;
 + unsigned long deadline;
 + unsigned int timeout;
   struct list_head timeout_list;
   int retries;

Can I suggest running 'pahole' over this when compiled on 64-bit?
You've just introduced a 4-byte hole.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
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