Date: Thu, 28 Sep 2017 11:54:25 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Chris Metcalf <cmetcalf@mellanox.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Luiz Capitulino <lcapitulino@redhat.com>,
        Christoph Lameter <cl@linux.com>,
        "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
        Mike Galbraith <efault@gmx.de>, Rik van Riel <riel@redhat.com>,
        Wanpeng Li <kernellwp@gmail.com>
Subject: Re: [GIT PULL] Introduce housekeeping subsystem v4
Message-ID: <20170928095425.2kcaehmnrukdkzfy@gmail.com>
References: <1505742849-30473-1-git-send-email-fweisbec@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1505742849-30473-1-git-send-email-fweisbec@gmail.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4943
Lines: 117


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the core/isolation-v4 branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	core/isolation-v4
> 
> HEAD: cf4c55aad44251369c8507c3823f9f9c51d4dc77
> 
> Summary of changes:
> 
> * Move the housekeeping code that was tied to NO_HZ to its own subsystem.
>   Currently NO_HZ governs the other isolation features which is not right
>   as dynticks is just an isolation feature like the others. We want to
>   centralize the CPU isolation decisions to a subsystem of its own instead.
> 
> * Integrate isolcpus code to housekeeping and treat it as a CPU isolation
>   feature.
> 
> * Reuse the "isolcpus=" kernel parameter to control the CPU isolation.
>   For now only tick and domains can be isolated after this patchset:
> 
>        isolcpus=1-7         # isolate domains on CPU range 1 to 7
>                             # "domain" flag is implicit by default to
>                             # keep the current behaviour
>                             
>        isolcpus=domain,1-7  # do the same
> 
>        isolcpus=nohz,1-7    # apply nohz_full to CPU range 1 to 7
> 
>        isolcpus=nohz,domain,1-7  # apply nohz_full and isolate domains of
>                                  # CPU range 1 to 7
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (12):
>       housekeeping: Move housekeeping related code to its own file
>       watchdog: Use housekeeping_cpumask() instead of ad-hoc version
>       housekeeping: Provide a dynamic off-case to housekeeping_any_cpu()
>       housekeeping: Make housekeeping cpumask private
>       housekeeping: Use its own static key
>       housekeeping: Rename is_housekeeping_cpu to housekeeping_cpu
>       housekeeping: Move it under its own config, independant from NO_HZ
>       housekeeping: Introduce housekeeping flags
>       housekeeping: Handle nohz_full= parameter
>       housekeeping: Move isolcpus to housekeeping
>       housekeeping: Add basic isolcpus flags
>       housekeeping: Document isolcpus flags
> 
> 
>  Documentation/admin-guide/kernel-parameters.txt |  33 +++---
>  drivers/base/cpu.c                              |  11 +-
>  drivers/net/ethernet/tile/tilegx.c              |   6 +-
>  include/linux/housekeeping.h                    |  51 ++++++++
>  include/linux/sched.h                           |   2 -
>  include/linux/tick.h                            |  39 +------
>  init/Kconfig                                    |   7 ++
>  init/main.c                                     |   2 +
>  kernel/Makefile                                 |   1 +
>  kernel/cgroup/cpuset.c                          |  15 +--
>  kernel/housekeeping.c                           | 149 ++++++++++++++++++++++++
>  kernel/rcu/tree_plugin.h                        |   3 +-
>  kernel/rcu/update.c                             |   3 +-
>  kernel/sched/core.c                             |  25 +---
>  kernel/sched/fair.c                             |   3 +-
>  kernel/sched/topology.c                         |  24 +---
>  kernel/time/tick-sched.c                        |  31 +----
>  kernel/watchdog.c                               |  13 +--
>  18 files changed, 276 insertions(+), 142 deletions(-)

Yeah, so while I agree that all this functionality needs to be factored
out and organized, I have a problem with this specific high level organization.

The main problem I think is that it's all called "housekeeping", which is pretty 
fuzzy and opaque. What does 'housekeeping' exactly mean? A dozen of details if you 
look at the code - and this name does not make it much easier to think about this 
whole problem category.

So how about introducing _two_ new high level concepts:

  1) 'global time handling'
  2) 'double async CPU callbacks'

The notion of 'global time' handling is obvious to everyone I think: it involves 
the system-global guarantee that certain kernel jobs will be executed 
periodically. At least one CPU in the system needs to handle 'global time'.

The notion of 'double async CPU callbacks' is less obvious: it involves the action 
of invoking a callback on a CPU, that might be executed on _another_ CPU.

I.e. there are 3 CPUs involved:

	- the invoking CPU
	- the target CPU
	- the CPU(s!) that will handle the callback (the housekeeping CPU mask)

For example the kmem-cache on_each_cpu() calls in mm/slab.c would fall into this 
category.

I don't know to what extent it makes sense to formalize and unify these 
facilities: it's certain that the (former) housekeeping CPU mask should be shared 
by these two facilities: the CPU executing global time callbacks periodically 
should be one of the CPUs that execute double-async CPU callbacks.

But by separating all this functionality into these two categories, it's already 
much easier to me to argue about which bit does what and why.

What do you think?

Thanks,

	Ingo