From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755150AbbCXThF (ORCPT ); Tue, 24 Mar 2015 15:37:05 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:44648 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752824AbbCXThB (ORCPT ); Tue, 24 Mar 2015 15:37:01 -0400 Message-ID: <5511BCDB.208@codeaurora.org> Date: Tue, 24 Mar 2015 12:36:59 -0700 From: Saravana Kannan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Thomas Gleixner CC: Joonwoo Park , linux-kernel@vger.kernel.org, John Stultz , Tejun Heo Subject: Re: [PATCH v2 RESEND/RFC] timer: make deferrable cpu unbound timers really not bound to a cpu References: <1410836376-27267-1-git-send-email-joonwoop@codeaurora.org> <550B6584.3020007@codeaurora.org> In-Reply-To: <550B6584.3020007@codeaurora.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/19/2015 05:10 PM, Saravana Kannan wrote: > On 09/23/2014 11:33 AM, Thomas Gleixner wrote: >> On Mon, 15 Sep 2014, Joonwoo Park wrote: >>> +#ifdef CONFIG_SMP >>> +static struct tvec_base *tvec_base_deferral = &boot_tvec_bases; >>> +#endif >> >> In principle I like the idea of a deferrable wheel, but this >> implementation is going to go nowhere. > > Hi Thomas, > > To give some more context: > > This bug is a serious pain in the a** for anyone using deferrable timers > or deferrable workqueues today for some periodic work and don't care for > which CPU the code runs in. > > Couple of examples of such issues in existing code: > > 1) In a SMP system, CPUfreq governors (ondemand and conservative) end up > queueing a deferrable work on every single CPU and the first one to run > the deferrable workqueue cancels the work on all other CPUS, runs the > work and then sets up a workqueue on all the CPUs again for the next > sampling point. > > 2) Devfreq actually doesn't work well today because it doesn't do this > nasty hack like CPUfreq. So, if the devfreq work happens to run on a CPU > that's typically idle, then it doesn't run for a long time. I've > actually seen logs where the devfreq polling interval is set to 20ms, > but ends up running 800ms later. > > Don't know how many other drivers are suffering from this bug. Yes, > IMHO, this is a bug. When the timer is CPU unbound, anyone who hasn't > looked at the actual timer code would assume that it'll run as long as > any CPU is busy. > >> First of all making it SMP only is silly. The deferrable stuff is a >> pain in other places as well. >> >> But whats way worse is: >> >>> +static inline void __run_timers(struct tvec_base *base, bool try) >>> { >>> struct timer_list *timer; >>> >>> - spin_lock_irq(&base->lock); >>> + if (!try) >>> + spin_lock_irq(&base->lock); >>> + else if (!spin_trylock_irq(&base->lock)) >>> + return; >> >> Yuck. All cpus fighting about a single spinlock roughly at the same >> time? You just created a proper thundering herd cacheline bouncing >> issue. > > I agree, This is not good. I think a simple way to fix this is to have > only the CPU that's the jiffy owner to run through this deferrable timer > list. > > That should address any unnecessary cache bouncing issues. Would that be > an acceptable solution to this? > >> >> No way. We have already mechanisms in place to deal with such >> problems, you just have to use them. > > I'm not sure which problem you are referring to here. Or what the > already existing solutions are. > > I don't think you were referring to the "deferrable timer getting > delayed for long periods despite CPUs being active" problem, because I > don't think we have a better solution than this patch (with the jiffy > owner CPU fix). Asking every driver that doesn't care which CPU the work > runs on to queue a work or set up a timer on every CPU is definitely not > a good solution -- that's spreading the complexity to every other driver > instead of fixing it in one place. And that causes unnecessary cache > pollution too. > > Thoughts? I would really like to see a fix for this go in soon so that > we can simplify cpufreq and have devfreq and other drivers work correctly. > > Thanks, > Saravana > Thomas, Bump. -Saravana -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project