Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933412Ab3GVUu5 (ORCPT ); Mon, 22 Jul 2013 16:50:57 -0400 Received: from smtp.codeaurora.org ([198.145.11.231]:44386 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756242Ab3GVUu4 (ORCPT ); Mon, 22 Jul 2013 16:50:56 -0400 Message-ID: <51ED9B2E.2030502@codeaurora.org> Date: Mon, 22 Jul 2013 13:50:54 -0700 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 MIME-Version: 1.0 To: John Stultz CC: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Thomas Gleixner , Russell King , Catalin Marinas , Will Deacon , Christopher Covington Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer References: <1374189690-10810-1-git-send-email-sboyd@codeaurora.org> <1374189690-10810-4-git-send-email-sboyd@codeaurora.org> <51ED781F.6060300@linaro.org> <51ED7DD5.8010000@codeaurora.org> <51ED80E7.4050606@codeaurora.org> <51ED9A94.3030204@linaro.org> In-Reply-To: <51ED9A94.3030204@linaro.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4001 Lines: 93 On 07/22/13 13:48, John Stultz wrote: > On 07/22/2013 11:58 AM, Stephen Boyd wrote: >> On 07/22/13 11:45, Stephen Boyd wrote: >>> On 07/22/13 11:21, John Stultz wrote: >>>> On 07/18/2013 04:21 PM, Stephen Boyd wrote: >>>>> In the next patch we're going to increase the number of bits that >>>>> the generic sched_clock can handle to be greater than 32. With >>>>> more than 32 bits the wraparound time can be larger than what can >>>>> fit into the units that msecs_to_jiffies takes (unsigned int). >>>>> Luckily, the wraparound is initially calculated in nanoseconds >>>>> which we can easily use with hrtimers, so switch to using an >>>>> hrtimer. >>>>> >>>>> Cc: Russell King >>>>> Signed-off-by: Stephen Boyd >>>> Hrmm. So in my testing (under qemu), this patch causes bootup to hang. >>>> >>>> qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9 >>>> -nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M >>>> raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB >>>> devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22 >>>> >>>> Config file attached. >>>> >>>> I haven't gotten a chance to look very closely, but it seems the >>>> folowing patch resolves the issue. I'm not sure if we're seeing >>>> callers to setup_sched_clock happen after sched_clock_postinit or >>>> what, but it probably needs another look over. >>>> >>>> thanks >>>> -john >>>> >>>> >>>> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c >>>> index a269890b..c018ffc 100644 >>>> --- a/kernel/time/sched_clock.c >>>> +++ b/kernel/time/sched_clock.c >>>> @@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void), >>>> int bits, unsigned long rate) >>>> pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns, >>>> wraps every %lluns\n", >>>> bits, r, r_unit, res, wrap); >>>> - /* >>>> - * Start the timer to keep sched_clock() properly updated and >>>> - * sets the initial epoch. >>>> - */ >>>> - hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, >>>> HRTIMER_MODE_REL); >>>> - sched_clock_timer.function = sched_clock_poll; >>>> update_sched_clock(); >>>> /* >>>> @@ -175,6 +169,13 @@ void __init sched_clock_postinit(void) >>>> setup_sched_clock(jiffy_sched_clock_read, 32, HZ); >>>> update_sched_clock(); >>>> + >>>> + /* >>>> + * Start the timer to keep sched_clock() properly updated and >>>> + * sets the initial epoch. >>>> + */ >>>> + hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, >>>> HRTIMER_MODE_REL); >>>> + sched_clock_timer.function = sched_clock_poll; >>>> hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL); >>>> } >>>> >>> Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into >>> sched_clock_register() also causes the same crash. >> Yes that seems to be the problem. The vexpress board is setting up the >> sched_clock in setup_arch() (via v2m_init_early) which runs before >> hrtimers_init(). I've only tested this on boards that setup the timer in >> the time_init() callback which runs after hrtimers_init(). Your patch >> should be fine, although it would be nice if we didn't have callers >> setting up the sched_clock so early. > > Although as Russell pointed out, setting up sched_clock later isn't > really a good option (although I'd like it best if we could handle > switching sched_clocks dynamically as needed so there were less > constraints on when it has to be registered). > > So I'll probably fold in my change into the patch if you're ok with that? > Yes that's fine to fold it in. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/