Re: [PATCH 4/8] cputime: Generic on-demand virtual cputime accounting

Hi Frederic,

On Mon, 28 Jan 2013 20:04:01 +0100 Frederic Weisbecker [off-list ref] wrote:

If we want to stop the tick further idle, we need to be
able to account the cputime without using the tick.

Virtual based cputime accounting solves that problem by
hooking into kernel/user boundaries.

However implementing CONFIG_VIRT_CPU_ACCOUNTING require
low level hooks and involves more overhead. But we already
have a generic context tracking subsystem that is required
for RCU needs by archs which plan to shut down the tick
outside idle.

This patch implements a generic virtual based cputime
accounting that relies on these generic kernel/user hooks.

There are some upsides of doing this:

- This requires no arch code to implement CONFIG_VIRT_CPU_ACCOUNTING
if context tracking is already built (already necessary for RCU in full
tickless mode).

- We can rely on the generic context tracking subsystem to dynamically
(de)activate the hooks, so that we can switch anytime between virtual
and tick based accounting. This way we don't have the overhead
of the virtual accounting when the tick is running periodically.

And one downside:

- There is probably more overhead than a native virtual based cputime
accounting. But this relies on hooks that are already set anyway.

Signed-off-by: Frederic Weisbecker <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Li Zhong <redacted>
Cc: Namhyung Kim <redacted>
Cc: Paul E. McKenney <redacted>
Cc: Paul Gortmaker <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <redacted>

This patch has the side effect of changing the default configurations:
(This is PowerPC pseries_defconfig before/after this patch)

@@ -119,8 +120,8 @@
 #
 # CPU/Task time and stats accounting
 #
-# CONFIG_TICK_CPU_ACCOUNTING is not set
-CONFIG_VIRT_CPU_ACCOUNTING=y
+CONFIG_TICK_CPU_ACCOUNTING=y
+# CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is not set
 # CONFIG_BSD_PROCESS_ACCT is not set
 CONFIG_TASKSTATS=y
 CONFIG_TASK_DELAY_ACCT=y

I don't know if that was deliberate, but it was suprising.  I noticed
when this patch entered next-20130207.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Hi Frederic,

On Fri, 8 Feb 2013 14:07:49 +1100 Stephen Rothwell [off-list ref] wrote:

I suspect that this is caused by the changes to init/Kconfig:

diff --git a/init/Kconfig b/init/Kconfig
index be8b7f5..a05f843 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -326,6 +326,9 @@ source "kernel/time/Kconfig"
 
 menu "CPU/Task time and stats accounting"
 
+config VIRT_CPU_ACCOUNTING
+	bool
+
 choice
 	prompt "Cputime accounting"
 	default TICK_CPU_ACCOUNTING if !PPC64

The next line of context is:

        default VIRT_CPU_ACCOUNTING if PPC64

Which may have needed changing as well?  Indeed, changing that to
VIRT_CPU_ACCOUNTING_NATIVE restores the old defaults.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Commit abf917cd91cb ("cputime: Generic on-demand virtual cputime
accounting") inadvertantly changed the default CPU_ACCOUNTING config
for PPC64.  Repair that.

Signed-off-by: Stephen Rothwell <redacted>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index ed24d1b..ccd1ca5 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -301,7 +301,7 @@ config VIRT_CPU_ACCOUNTING
 choice
 	prompt "Cputime accounting"
 	default TICK_CPU_ACCOUNTING if !PPC64
-	default VIRT_CPU_ACCOUNTING if PPC64
+	default VIRT_CPU_ACCOUNTING_NATIVE if PPC64
 
 # Kind of a stub config for the pure tick based cputime accounting
 config TICK_CPU_ACCOUNTING

-- 
1.8.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Commit-ID:  02fc8d37229d15c654876cf9ce56b5c1cf7945d7
Gitweb:     http://git.kernel.org/tip/02fc8d37229d15c654876cf9ce56b5c1cf7945d7
Author:     Stephen Rothwell [off-list ref]
AuthorDate: Fri, 8 Feb 2013 14:19:38 +1100
Committer:  Ingo Molnar [off-list ref]
CommitDate: Fri, 8 Feb 2013 15:23:12 +0100

cputime: Restore CPU_ACCOUNTING config defaults for PPC64

Commit abf917cd91cb ("cputime: Generic on-demand virtual cputime
accounting") inadvertantly changed the default CPU_ACCOUNTING
config for PPC64.  Repair that.

Signed-off-by: Stephen Rothwell <redacted>
Acked-by: Frederic Weisbecker <redacted>
Cc: Li Zhong <redacted>
Cc: Namhyung Kim <redacted>
Cc: Paul E. McKenney <redacted>
Cc: Paul Gortmaker <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: ppc-dev <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Link: http://lkml.kernel.org/r/20130208141938.f31b7b9e1acac5bbe769ee4c@canb.auug.org.au
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index a05f843..ccb9f8f 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -332,7 +332,7 @@ config VIRT_CPU_ACCOUNTING
 choice
 	prompt "Cputime accounting"
 	default TICK_CPU_ACCOUNTING if !PPC64
-	default VIRT_CPU_ACCOUNTING if PPC64
+	default VIRT_CPU_ACCOUNTING_NATIVE if PPC64
 
 # Kind of a stub config for the pure tick based cputime accounting
 config TICK_CPU_ACCOUNTING

2013/2/8 Stephen Rothwell [off-list ref]:

Indeed, thanks for your fix!