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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

Hi Frederic,

On Mon, 28 Jan 2013 20:04:01 +0100 Frederic Weisbecker <fweisbec@gmail.com> 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 <fweisbec@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Li Zhong <zhong@linux.vnet.ibm.com>
> Cc: Namhyung Kim <namhyung.kim@lge.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

Hi Frederic,

On Fri, 8 Feb 2013 14:07:49 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> 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.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] cputime: restore CPU_ACCOUNTING config defaults for PPC64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

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 <sfr@canb.auug.org.au>
---
 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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Frederic Weisbecker]

2013/2/8 Stephen Rothwell <sfr@canb.auug.org.au>:
> Hi Frederic,
>
> On Fri, 8 Feb 2013 14:07:49 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> 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.
>
> 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.

Indeed, thanks for your fix!

[tip:sched/core] cputime: Restore CPU_ACCOUNTING config defaults for PPC64

[tip-bot for Stephen Rothwell]

Commit-ID:  02fc8d37229d15c654876cf9ce56b5c1cf7945d7
Gitweb:     http://git.kernel.org/tip/02fc8d37229d15c654876cf9ce56b5c1cf7945d7
Author:     Stephen Rothwell <sfr@canb.auug.org.au>
AuthorDate: Fri, 8 Feb 2013 14:19:38 +1100
Committer:  Ingo Molnar <mingo@kernel.org>
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 <sfr@canb.auug.org.au>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: ppc-dev <linuxppc-dev@lists.ozlabs.org>
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