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