From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S264304AbTDKHQl (for ); Fri, 11 Apr 2003 03:16:41 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S264305AbTDKHQl (for ); Fri, 11 Apr 2003 03:16:41 -0400 Received: from [12.47.58.73] ([12.47.58.73]:44671 "EHLO pao-ex01.pao.digeo.com") by vger.kernel.org with ESMTP id S264304AbTDKHQj (for ); Fri, 11 Apr 2003 03:16:39 -0400 Date: Fri, 11 Apr 2003 00:28:16 -0700 From: Andrew Morton To: davidm@hpl.hp.com Cc: linux-kernel@vger.kernel.org, george anzinger Subject: Re: too much timer simplification... Message-Id: <20030411002816.786296e8.akpm@digeo.com> In-Reply-To: <200304110705.h3B75aQt026081@napali.hpl.hp.com> References: <200304110705.h3B75aQt026081@napali.hpl.hp.com> X-Mailer: Sylpheed version 0.8.9 (GTK+ 1.2.10; i586-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 11 Apr 2003 07:28:15.0463 (UTC) FILETIME=[EA6C3770:01C2FFFB] Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org David Mosberger wrote: > > It appears to me that this changeset: > > http://linux.bkbits.net:8080/linux-2.5/diffs/kernel/timer.c@1.48 > > may have gone a little too far. > > What I'm seeing is that if someone happens to arm a periodic timer at > exactly 256 jiffies (as ohci happens to do on platforms with HZ=1024), > then you end up getting an endless loop of timer activations, causing > a machine hang. > > The problem is that __run_timers updates base->timer_jiffies _before_ > running the callback routines. If a callback re-arms the timer at > exactly 256 jiffies, add_timers() will reinsert the timer into the > list that we're currently processing, which of course will cause the > timer to expire immediately again, etc., etc., ad naseum... > OK, well unless George can pull a rabbit out of the hat it may be best to just revert it. This gives us the same algorithm as 2.4, which I think is good. --- 25/kernel/timer.c~timer-simplification-revert 2003-04-11 00:19:48.000000000 -0700 +++ 25-akpm/kernel/timer.c 2003-04-11 00:19:48.000000000 -0700 @@ -56,6 +56,7 @@ struct tvec_t_base_s { spinlock_t lock; unsigned long timer_jiffies; struct timer_list *running_timer; + struct list_head *run_timer_list_running; tvec_root_t tv1; tvec_t tv2; tvec_t tv3; @@ -100,6 +101,12 @@ static inline void check_timer(struct ti check_timer_failed(timer); } +/* + * If a timer handler re-adds the timer with expires == jiffies, the timer + * running code can lock up. So here we detect that situation and park the + * timer onto base->run_timer_list_running. It will be added to the main timer + * structures later, by __run_timers(). + */ static void internal_add_timer(tvec_base_t *base, struct timer_list *timer) { @@ -107,7 +114,9 @@ static void internal_add_timer(tvec_base unsigned long idx = expires - base->timer_jiffies; struct list_head *vec; - if (idx < TVR_SIZE) { + if (base->run_timer_list_running) { + vec = base->run_timer_list_running; + } else if (idx < TVR_SIZE) { int i = expires & TVR_MASK; vec = base->tv1.vec + i; } else if (idx < 1 << (TVR_BITS + TVN_BITS)) { @@ -397,6 +406,7 @@ static inline void __run_timers(tvec_bas spin_lock_irq(&base->lock); while (time_after_eq(jiffies, base->timer_jiffies)) { + LIST_HEAD(deferred_timers); struct list_head *head; int index = base->timer_jiffies & TVR_MASK; @@ -408,7 +418,7 @@ static inline void __run_timers(tvec_bas (!cascade(base, &base->tv3, INDEX(1))) && !cascade(base, &base->tv4, INDEX(2))) cascade(base, &base->tv5, INDEX(3)); - ++base->timer_jiffies; + base->run_timer_list_running = &deferred_timers; repeat: head = base->tv1.vec + index; if (!list_empty(head)) { @@ -427,6 +437,14 @@ repeat: spin_lock_irq(&base->lock); goto repeat; } + base->run_timer_list_running = NULL; + ++base->timer_jiffies; + while (!list_empty(&deferred_timers)) { + timer = list_entry(deferred_timers.prev, + struct timer_list, entry); + list_del(&timer->entry); + internal_add_timer(base, timer); + } } set_running_timer(base, NULL); spin_unlock_irq(&base->lock); _