From patchwork Tue Jun 7 18:29:08 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 859012 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p57ITibh018008 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 7 Jun 2011 18:30:06 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QU11t-0008B2-Jl; Tue, 07 Jun 2011 18:29:29 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QU11t-0007oj-6o; Tue, 07 Jun 2011 18:29:29 +0000 Received: from relais.videotron.ca ([24.201.245.36]) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QU11p-0007oQ-Gl for linux-arm-kernel@lists.infradead.org; Tue, 07 Jun 2011 18:29:26 +0000 MIME-version: 1.0 Received: from xanadu.home ([66.130.28.92]) by vl-mo-mrz23.ip.videotron.ca (Sun Java(tm) System Messaging Server 6.3-8.01 (built Dec 16 2008; 32bit)) with ESMTP id <0LMF00DBANBC5H90@vl-mo-mrz23.ip.videotron.ca> for linux-arm-kernel@lists.infradead.org; Tue, 07 Jun 2011 14:28:25 -0400 (EDT) Date: Tue, 07 Jun 2011 14:29:08 -0400 (EDT) From: Nicolas Pitre X-X-Sender: nico@xanadu.home To: Lennert Buytenhek Subject: Re: [PATCH,RFC] mmp clockevent handling race In-reply-to: <20110607140456.GE11275@wantstofly.org> Message-id: References: <20110607140456.GE11275@wantstofly.org> User-Agent: Alpine 2.00 (LFD 1167 2008-08-23) X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110607_142925_591847_F4B20EBD X-CRM114-Status: GOOD ( 26.33 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, low trust [24.201.245.36 listed in list.dnswl.org] Cc: martin@laptop.org, cjb@laptop.org, Eric Miao , Haojian Zhuang , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 07 Jun 2011 18:30:06 +0000 (UTC) On Tue, 7 Jun 2011, Lennert Buytenhek wrote: > Hi! > > The patch below has been sitting around in the OLPC XO-1.75 (MMP2-based > XO) bringup kernel tree for some time now, and upon recent rebasing of > this tree to 3.0, it was discovered that something like this patch is > still needed. > > Symptoms: e.g. as described here: http://dev.laptop.org/ticket/10945 > -- applications hang for a couple of minutes at a time, and sometimes > there are several-minute hangs during the boot process. > > >From the ticket: > > The problem in the current upstream mmp timer handling code > that appears to be triggered here is that it handles clockevents > by setting up a comparator on the free-running clocksource timer > to match and trigger an interrupt at 'current_time + delta', > which if delta is small enough can lead to 'current_time + delta' > already being in the past when comparator setup has finished, > and the requested event will not trigger. This is a classical issue that was solved on the SA1100 more than 10 years ago. > What this patch does is to rewrite the timer handling code to use > two timers, one for the free-running clocksource, and one to trigger > clockevents with, which is more or less the standard approach to this. > It's kind of invasive, though (certainly more invasive than it strictly > needs to be, as it reorganises time.c somewhat at the same time), and > something like this is probably too invasive for 3.0 at this point. > > A safer "fix" for 3.0 may be to disallow NO_HZ/HIGH_RES_TIMERS on mmp. > > Any thoughts? What about simply this: Nicolas diff --git a/arch/arm/mach-mmp/time.c b/arch/arm/mach-mmp/time.c index 99833b9..8b8f99a 100644 --- a/arch/arm/mach-mmp/time.c +++ b/arch/arm/mach-mmp/time.c @@ -39,7 +39,7 @@ #define TIMERS_VIRT_BASE TIMERS1_VIRT_BASE -#define MAX_DELTA (0xfffffffe) +#define MAX_DELTA (0xfffffffe - 16) #define MIN_DELTA (16) static DEFINE_CLOCK_DATA(cd); @@ -85,7 +85,7 @@ static irqreturn_t timer_interrupt(int irq, void *dev_id) static int timer_set_next_event(unsigned long delta, struct clock_event_device *dev) { - unsigned long flags, next; + unsigned long flags, next, now; local_irq_save(flags); @@ -95,9 +95,10 @@ static int timer_set_next_event(unsigned long delta, next = timer_read() + delta; __raw_writel(next, TIMERS_VIRT_BASE + TMR_TN_MM(0, 0)); + now = timer_read(); local_irq_restore(flags); - return 0; + return (signed)(next - now) <= MIN_DELTA ? -ETIME : 0; } static void timer_set_mode(enum clock_event_mode mode,