[PATCH 0/3] nohz/full: final Kconfig bits for ARM support

[PATCH 0/3] nohz/full: final Kconfig bits for ARM support

[Kevin Hilman]

As of v3.10, the core ARM support is in mainline for NO_HZ_FULL.  The
only remaining part is the removal of the hard-coded Kconfig
requirement on 64-bit platforms, which I believe can now be removed
after the nsec granularity cputime was made to work on non 64_BIT
(c.f. commit 8c23b80e, cputime_nsecs: use math64.h for nsec
resolution conversion helpers.)

This series makes the final Kconfig changes to bring NO_HZ_FULL
support to ARM.

Series applies to v3.10.

I will queue up the arch/arm patch for Russell separately once the
generic changes are accepted.

Kevin

 arch/arm/Kconfig    | 1 +
 init/Kconfig        | 2 +-
 kernel/time/Kconfig | 2 --
 3 files changed, 2 insertions(+), 3 deletions(-)

-- 
1.8.3

[PATCH 1/3] nohz: Kconfig: remove redundant 64-bit dependency

[Kevin Hilman]

The current 64-bit dependency is on VIRT_CPU_ACCOUNTING_GEN, which already
has its own dependency on 64-bit.  Remove it from NO_HZ_FULL.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/time/Kconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 70f27e8..b78760f 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -99,8 +99,6 @@ config NO_HZ_FULL
 	depends on SMP
 	# RCU_USER_QS dependency
 	depends on HAVE_CONTEXT_TRACKING
-	# VIRT_CPU_ACCOUNTING_GEN dependency
-	depends on 64BIT
 	select NO_HZ_COMMON
 	select RCU_USER_QS
 	select RCU_NOCB_CPU
-- 
1.8.3

[PATCH 1/3] nohz: Kconfig: remove redundant 64-bit dependency

[Frederic Weisbecker]

On Wed, Jul 03, 2013 at 11:36:39AM -0700, Kevin Hilman wrote:
> The current 64-bit dependency is on VIRT_CPU_ACCOUNTING_GEN, which already
> has its own dependency on 64-bit.  Remove it from NO_HZ_FULL.

It's not exactly redundant. It's a necessary dependency inheritance.
When we select NO_HZ_FULL, TICK_CPU_ACCOUNTING and VIRT_CPU_ACCOUNTING_NATIVE
disappear such that only VIRT_CPU_ACCOUNTING_GEN can be selected. But if we
don't have CONFIG_64bits, then VIRT_CPU_ACCOUNTING_GEN is not possible either.
So we end up with nothing for the accounting (or whatever random reaction
from Kconfig against that situation) and that's a bug. So we need to inherit the
64 bits dependency on NO_HZ_FULL to prevent that.

We can remove this dependency from NO_HZ_FULL, but only once we remove it
from VIRT_CPU_ACCOUNTING_GEN, not the other way around otherwise this
breaks bisection.

> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/time/Kconfig | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
> index 70f27e8..b78760f 100644
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -99,8 +99,6 @@ config NO_HZ_FULL
>  	depends on SMP
>  	# RCU_USER_QS dependency
>  	depends on HAVE_CONTEXT_TRACKING
> -	# VIRT_CPU_ACCOUNTING_GEN dependency
> -	depends on 64BIT
>  	select NO_HZ_COMMON
>  	select RCU_USER_QS
>  	select RCU_NOCB_CPU
> -- 
> 1.8.3
> 

[PATCH 2/3] init/Kconfig: VIRT_CPU_ACCOUNTING_GEN: drop 64-bit requirement

[Kevin Hilman]

The 64-bit requirement can be removed after the conversion of the nsec
granularity cputime to work on !64_BIT, which was done in commit
8c23b80e (cputime_nsecs: use math64.h for nsec resolution conversion
helpers)

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 2d9b831..e151022 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -325,7 +325,7 @@ config VIRT_CPU_ACCOUNTING_NATIVE
 
 config VIRT_CPU_ACCOUNTING_GEN
 	bool "Full dynticks CPU time accounting"
-	depends on HAVE_CONTEXT_TRACKING && 64BIT
+	depends on HAVE_CONTEXT_TRACKING
 	select VIRT_CPU_ACCOUNTING
 	select CONTEXT_TRACKING
 	help
-- 
1.8.3

[PATCH 2/3] init/Kconfig: VIRT_CPU_ACCOUNTING_GEN: drop 64-bit requirement

[Frederic Weisbecker]

On Wed, Jul 03, 2013 at 11:36:40AM -0700, Kevin Hilman wrote:
> The 64-bit requirement can be removed after the conversion of the nsec
> granularity cputime to work on !64_BIT, which was done in commit
> 8c23b80e (cputime_nsecs: use math64.h for nsec resolution conversion
> helpers)
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

So I finally sat down and checked all the use of cputime_t (I probably
missed some) to make sure that readers are safe against concurrent write
in 32 bits archs.

And everything looks ok expect perhaps fs/[compat]binfmt_elf*.c that doesn't
lock the signal struct before getting tsk->signal->cutime and stuff. But perhaps
it's safe for some reason, I'll look more carefully. And I also need to
check further itimers code.

But I think I pushed back the ARM support long enough now so it's time to
get this patch in. The above issues are mostly details that can be dealt
with separately.

Just one thing before we apply this. I haven't checked the arch/* potential
unsafe uses of cputime. So it would be nice to bring a new
CONFIG_HAVE_VIRT_CPU_ACCOUNTING_GEN. This won't require any specific arch
support, it would just be there to inform the arch developer, through the help
text, about the issue with cputime being u64 and resulting possible race.
So the guy can have a look at cputime_t uses in his arch before finally enabling
that config.

Also, of course check that arm is fine wrt. that and finally enable it :)

Thanks!


> ---
>  init/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 2d9b831..e151022 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -325,7 +325,7 @@ config VIRT_CPU_ACCOUNTING_NATIVE
>  
>  config VIRT_CPU_ACCOUNTING_GEN
>  	bool "Full dynticks CPU time accounting"
> -	depends on HAVE_CONTEXT_TRACKING && 64BIT
> +	depends on HAVE_CONTEXT_TRACKING
>  	select VIRT_CPU_ACCOUNTING
>  	select CONTEXT_TRACKING
>  	help
> -- 
> 1.8.3
> 

[PATCH 3/3] ARM: Kconfig: allow virt CPU accounting

[Kevin Hilman]

With the 64-bit requirement removed from virt CPU accounting,
allow ARM platforms to enable it.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 136f263..7850612 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -61,6 +61,7 @@ config ARM
 	select OLD_SIGSUSPEND3
 	select OLD_SIGACTION
 	select HAVE_CONTEXT_TRACKING
+	select HAVE_VIRT_CPU_ACCOUNTING
 	help
 	  The ARM series is a line of low-power-consumption RISC chip designs
 	  licensed by ARM Ltd and targeted at embedded applications and
-- 
1.8.3

[PATCH 3/3] ARM: Kconfig: allow virt CPU accounting

[Frederic Weisbecker]

On Wed, Jul 03, 2013 at 11:36:41AM -0700, Kevin Hilman wrote:
> With the 64-bit requirement removed from virt CPU accounting,
> allow ARM platforms to enable it.
> 
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  arch/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 136f263..7850612 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -61,6 +61,7 @@ config ARM
>  	select OLD_SIGSUSPEND3
>  	select OLD_SIGACTION
>  	select HAVE_CONTEXT_TRACKING
> +	select HAVE_VIRT_CPU_ACCOUNTING

I think you got confused here. HAVE_VIRT_CPU_ACCOUNTING is the arch capability
for VIRT_CPU_ACCOUNTING_NATIVE, not for VIRT_CPU_ACCOUNTING_GEN that only requires
support for context tracking.

That's my bad, all those names start to be confusing now.
The VIRT based Kconfig naming is a bit weird, that doesn't really reflect what the feature
is doing. "Virtual cputime accounting" just doesn't give any clue, except perhaps suggesting
the stuff deals with virtualization while it actually has nothing to do with.
I don't even know what virtual refers to here.

Same goes for vtime based APIs. In fact I just based my work on the legacy that was
there and expanded further the non-sense ;-)

I'll need to do a big renaming one day.

But to begin with I should rename s/HAVE_VIRT_CPU_ACCOUNTING/HAVE_VIRT_CPU_ACCOUNTING_NATIVE.



>  	help
>  	  The ARM series is a line of low-power-consumption RISC chip designs
>  	  licensed by ARM Ltd and targeted at embedded applications and
> -- 
> 1.8.3
> 

[PATCH 3/3] ARM: Kconfig: allow virt CPU accounting

[Kevin Hilman]

Frederic Weisbecker <fweisbec@gmail.com> writes:

> On Wed, Jul 03, 2013 at 11:36:41AM -0700, Kevin Hilman wrote:
>> With the 64-bit requirement removed from virt CPU accounting,
>> allow ARM platforms to enable it.
>> 
>> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  arch/arm/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 136f263..7850612 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -61,6 +61,7 @@ config ARM
>>  	select OLD_SIGSUSPEND3
>>  	select OLD_SIGACTION
>>  	select HAVE_CONTEXT_TRACKING
>> +	select HAVE_VIRT_CPU_ACCOUNTING
>
> I think you got confused here. HAVE_VIRT_CPU_ACCOUNTING is the arch capability
> for VIRT_CPU_ACCOUNTING_NATIVE, not for VIRT_CPU_ACCOUNTING_GEN that only requires
> support for context tracking.

Yes, I am confused. :/

> That's my bad, all those names start to be confusing now.
> The VIRT based Kconfig naming is a bit weird, that doesn't really reflect what the feature
> is doing. "Virtual cputime accounting" just doesn't give any clue, except perhaps suggesting
> the stuff deals with virtualization while it actually has nothing to do with.
> I don't even know what virtual refers to here.
>
> Same goes for vtime based APIs. In fact I just based my work on the legacy that was
> there and expanded further the non-sense ;-)
>
> I'll need to do a big renaming one day.
>
> But to begin with I should rename s/HAVE_VIRT_CPU_ACCOUNTING/HAVE_VIRT_CPU_ACCOUNTING_NATIVE.

OK, I just tested and I can indeed drop this patch.

Thanks for clarifying,

Kevin