Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753667AbcCHNOx (ORCPT ); Tue, 8 Mar 2016 08:14:53 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36506 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753470AbcCHNOd (ORCPT ); Tue, 8 Mar 2016 08:14:33 -0500 Date: Tue, 8 Mar 2016 14:14:23 +0100 From: Ingo Molnar To: Frederic Weisbecker Cc: LKML , Peter Zijlstra , Chris Metcalf , Thomas Gleixner , Luiz Capitulino , Christoph Lameter , Viresh Kumar , Rik van Riel Subject: Re: [GIT PULL] nohz: Tick dependency mask v2 Message-ID: <20160308131423.GA5646@gmail.com> References: <1454605255-23796-1-git-send-email-fweisbec@gmail.com> <1454605255-23796-4-git-send-email-fweisbec@gmail.com> <20160216080345.GA27908@gmail.com> <20160303004658.GA1749@lerouge> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160303004658.GA1749@lerouge> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3820 Lines: 108 * Frederic Weisbecker wrote: > On Tue, Feb 16, 2016 at 09:03:45AM +0100, Ingo Molnar wrote: > > > > > > So I think it would be useful to name this in a way the expresses that this is a > > mask. > > > > 'tick_dep_mask' or so? > > [...] > > > > > > > +enum tick_dependency_bit { > > > > s/tick_dep_bits > > > > > + TICK_POSIX_TIMER_BIT = 0, > > > + TICK_PERF_EVENTS_BIT = 1, > > > + TICK_SCHED_BIT = 2, > > > + TICK_CLOCK_UNSTABLE_BIT = 3 > > > > s/TICK_DEP_BIT_... > > > > > +}; > > > + > > > +#define TICK_POSIX_TIMER_MASK (1 << TICK_POSIX_TIMER_BIT) > > > +#define TICK_PERF_EVENTS_MASK (1 << TICK_PERF_EVENTS_BIT) > > > +#define TICK_SCHED_MASK (1 << TICK_SCHED_BIT) > > > +#define TICK_CLOCK_UNSTABLE_MASK (1 << TICK_CLOCK_UNSTABLE_BIT) > > > > So I'd rename this to: > > > > #define TICK_DEP_MASK_POSIX_TIMER (1 << TICK_POSIX_TIMER_BIT) > > #define TICK_DEP_MASK_PERF_EVENTS (1 << TICK_PERF_EVENTS_BIT) > > #define TICK_DEP_MASK_SCHED (1 << TICK_SCHED_BIT) > > #define TICK_DEP_MASK_CLOCK_UNSTABLE (1 << TICK_CLOCK_UNSTABLE_BIT) > > > > i.e. the 'tick_dep' and 'TICK_DEP' nomenclature would be used throughout the code > > and the pattern would be easy to grep for. > > > > > +extern void tick_nohz_set_dep(enum tick_dependency_bit bit); > > > +extern void tick_nohz_clear_dep(enum tick_dependency_bit bit); > > > +extern void tick_nohz_set_dep_cpu(int cpu, enum tick_dependency_bit bit); > > > +extern void tick_nohz_clear_dep_cpu(int cpu, enum tick_dependency_bit bit); > > > +extern void tick_nohz_set_dep_task(struct task_struct *tsk, > > > + enum tick_dependency_bit bit); > > > +extern void tick_nohz_clear_dep_task(struct task_struct *tsk, > > > + enum tick_dependency_bit bit); > > > +extern void tick_nohz_set_dep_signal(struct signal_struct *signal, > > > + enum tick_dependency_bit bit); > > > +extern void tick_nohz_clear_dep_signal(struct signal_struct *signal, > > > + enum tick_dependency_bit bit); > > > > Ditto, please rename it all to: > > > > tick_dep_set() > > tick_dep_clear() > > tick_dep_set_cpu() > > tick_dep_clear_cpu() > > tick_dep_set_task() > > ... > > Ok, I fixed all the above. > > > > > also, please don't line-break function prototypes, it only makes the result harder > > to read. > > I couldn't fix that though, I'm limited by the 80 columns. > > > If you're ok with it, please pull the branch: > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git > timers/core-v9 > > HEAD: 4f49b90abb4aca6fe677c95fc352fd0674d489bd > > --- Summary --- > > Currently in nohz full configs, the tick dependency is checked > asynchronously by nohz code from interrupt and context switch for each > concerned subsystem with a set of function provided by these. Such > functions are made of many conditions and details that can be heavyweight > as they are called on fastpath: sched_can_stop_tick(), > posix_cpu_timer_can_stop_tick(), perf_event_can_stop_tick()... > > Thomas suggested a few month ago to make that tick dependency check > synchronous. Instead of checking subsystems details from each interrupt > to guess if the tick can be stopped, every subsystem that may have a tick > dependency should set itself a flag specifying the state of that > dependency. This way we can verify if we can stop the tick with a single > lightweight mask check on fast path. > > This conversion from a pull to a push model to implement tick dependency > is the core feature of this patchset that is split into: > > * Nohz wide kick simplification > * Improve nohz tracing > * Introduce tick dependency mask > * Migrate scheduler, posix timers, perf events and sched clock tick > dependencies to the tick dependency mask. Pulled into tip:timers/nohz, thanks a lot Frederic! Ingo