LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests/cpufreq: Don't enable generic lock debugging options
@ 2023-06-06 14:11 Mark Brown
  2023-06-07  3:45 ` Viresh Kumar
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2023-06-06 14:11 UTC (permalink / raw)
  To: Rafael J. Wysocki, Viresh Kumar, Shuah Khan
  Cc: linux-pm, linux-kselftest, linux-kernel, Mark Brown

Currently the the config fragment for cpufreq enables a lot of generic
lock debugging.  While these options are useful when testing cpufreq
they aren't actually required to run the tests and are therefore out of
scope for the cpufreq fragement, they are more of a thing that it's good
to enable while doing testing than an actual requirement for cpufreq
testing specifically.  Having these debugging options enabled,
especially the mutex and spinlock instrumentation, mean that any build
that includes the cpufreq fragment is both very much larger than a
standard defconfig (eg, I'm seeing 35% on x86_64) and also slower at
runtime.

This is causing real problems for CI systems.  In order to avoid
building large numbers of kernels they try to group kselftest fragments
together, frequently just grouping all the kselftest fragments into a
single block.  The increased size is an issue for memory constrained
systems and is also problematic for systems with fixed storage
allocations for kernel images (eg, typical u-boot systems) where it
frequently causes the kernel to overflow the storage space allocated for
kernels.  The reduced performance isn't too bad with real hardware but
can be disruptive on emulated platforms.

In order to avoid these issues remove these generic instrumentation
options from the cpufreq fragment, bringing the cpufreq fragment into
line with other fragments which generally set requirements for testing
rather than nice to haves.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/cpufreq/config | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tools/testing/selftests/cpufreq/config b/tools/testing/selftests/cpufreq/config
index 75e900793e8a..ce5068f5a6a2 100644
--- a/tools/testing/selftests/cpufreq/config
+++ b/tools/testing/selftests/cpufreq/config
@@ -5,11 +5,3 @@ CONFIG_CPU_FREQ_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y
 CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
-CONFIG_DEBUG_RT_MUTEXES=y
-CONFIG_DEBUG_PLIST=y
-CONFIG_DEBUG_SPINLOCK=y
-CONFIG_DEBUG_MUTEXES=y
-CONFIG_DEBUG_LOCK_ALLOC=y
-CONFIG_PROVE_LOCKING=y
-CONFIG_LOCKDEP=y
-CONFIG_DEBUG_ATOMIC_SLEEP=y

---
base-commit: ac9a78681b921877518763ba0e89202254349d1b
change-id: 20230605-kselftest-cpufreq-options-2fd6d4742333

Best regards,
-- 
Mark Brown <broonie@kernel.org>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests/cpufreq: Don't enable generic lock debugging options
  2023-06-06 14:11 [PATCH] selftests/cpufreq: Don't enable generic lock debugging options Mark Brown
@ 2023-06-07  3:45 ` Viresh Kumar
  2023-06-09 18:52   ` Shuah Khan
  0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2023-06-07  3:45 UTC (permalink / raw)
  To: Mark Brown
  Cc: Rafael J. Wysocki, Shuah Khan, linux-pm, linux-kselftest,
	linux-kernel

On 06-06-23, 15:11, Mark Brown wrote:
> Currently the the config fragment for cpufreq enables a lot of generic
> lock debugging.  While these options are useful when testing cpufreq
> they aren't actually required to run the tests and are therefore out of
> scope for the cpufreq fragement, they are more of a thing that it's good
> to enable while doing testing than an actual requirement for cpufreq
> testing specifically.  Having these debugging options enabled,
> especially the mutex and spinlock instrumentation, mean that any build
> that includes the cpufreq fragment is both very much larger than a
> standard defconfig (eg, I'm seeing 35% on x86_64) and also slower at
> runtime.
> 
> This is causing real problems for CI systems.  In order to avoid
> building large numbers of kernels they try to group kselftest fragments
> together, frequently just grouping all the kselftest fragments into a
> single block.  The increased size is an issue for memory constrained
> systems and is also problematic for systems with fixed storage
> allocations for kernel images (eg, typical u-boot systems) where it
> frequently causes the kernel to overflow the storage space allocated for
> kernels.  The reduced performance isn't too bad with real hardware but
> can be disruptive on emulated platforms.
> 
> In order to avoid these issues remove these generic instrumentation
> options from the cpufreq fragment, bringing the cpufreq fragment into
> line with other fragments which generally set requirements for testing
> rather than nice to haves.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  tools/testing/selftests/cpufreq/config | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/cpufreq/config b/tools/testing/selftests/cpufreq/config
> index 75e900793e8a..ce5068f5a6a2 100644
> --- a/tools/testing/selftests/cpufreq/config
> +++ b/tools/testing/selftests/cpufreq/config
> @@ -5,11 +5,3 @@ CONFIG_CPU_FREQ_GOV_USERSPACE=y
>  CONFIG_CPU_FREQ_GOV_ONDEMAND=y
>  CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y
>  CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
> -CONFIG_DEBUG_RT_MUTEXES=y
> -CONFIG_DEBUG_PLIST=y
> -CONFIG_DEBUG_SPINLOCK=y
> -CONFIG_DEBUG_MUTEXES=y
> -CONFIG_DEBUG_LOCK_ALLOC=y
> -CONFIG_PROVE_LOCKING=y
> -CONFIG_LOCKDEP=y
> -CONFIG_DEBUG_ATOMIC_SLEEP=y

FWIW, I enabled these earlier as cpufreq core had a history of races
that are normally not caught without these enabled. But I think we
have come a long way from that and these can be removed now.

-- 
viresh

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests/cpufreq: Don't enable generic lock debugging options
  2023-06-07  3:45 ` Viresh Kumar
@ 2023-06-09 18:52   ` Shuah Khan
  0 siblings, 0 replies; 3+ messages in thread
From: Shuah Khan @ 2023-06-09 18:52 UTC (permalink / raw)
  To: Viresh Kumar, Mark Brown
  Cc: Rafael J. Wysocki, Shuah Khan, linux-pm, linux-kselftest,
	linux-kernel, Shuah Khan

On 6/6/23 21:45, Viresh Kumar wrote:
> On 06-06-23, 15:11, Mark Brown wrote:
>> Currently the the config fragment for cpufreq enables a lot of generic
>> lock debugging.  While these options are useful when testing cpufreq
>> they aren't actually required to run the tests and are therefore out of
>> scope for the cpufreq fragement, they are more of a thing that it's good
>> to enable while doing testing than an actual requirement for cpufreq
>> testing specifically.  Having these debugging options enabled,
>> especially the mutex and spinlock instrumentation, mean that any build
>> that includes the cpufreq fragment is both very much larger than a
>> standard defconfig (eg, I'm seeing 35% on x86_64) and also slower at
>> runtime.
>>
>> This is causing real problems for CI systems.  In order to avoid
>> building large numbers of kernels they try to group kselftest fragments
>> together, frequently just grouping all the kselftest fragments into a
>> single block.  The increased size is an issue for memory constrained
>> systems and is also problematic for systems with fixed storage
>> allocations for kernel images (eg, typical u-boot systems) where it
>> frequently causes the kernel to overflow the storage space allocated for
>> kernels.  The reduced performance isn't too bad with real hardware but
>> can be disruptive on emulated platforms.
>>
>> In order to avoid these issues remove these generic instrumentation
>> options from the cpufreq fragment, bringing the cpufreq fragment into
>> line with other fragments which generally set requirements for testing
>> rather than nice to haves.
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>   
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> ---
>>   tools/testing/selftests/cpufreq/config | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/cpufreq/config b/tools/testing/selftests/cpufreq/config
>> index 75e900793e8a..ce5068f5a6a2 100644
>> --- a/tools/testing/selftests/cpufreq/config
>> +++ b/tools/testing/selftests/cpufreq/config
>> @@ -5,11 +5,3 @@ CONFIG_CPU_FREQ_GOV_USERSPACE=y
>>   CONFIG_CPU_FREQ_GOV_ONDEMAND=y
>>   CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y
>>   CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
>> -CONFIG_DEBUG_RT_MUTEXES=y
>> -CONFIG_DEBUG_PLIST=y
>> -CONFIG_DEBUG_SPINLOCK=y
>> -CONFIG_DEBUG_MUTEXES=y
>> -CONFIG_DEBUG_LOCK_ALLOC=y
>> -CONFIG_PROVE_LOCKING=y
>> -CONFIG_LOCKDEP=y
>> -CONFIG_DEBUG_ATOMIC_SLEEP=y
> 
> FWIW, I enabled these earlier as cpufreq core had a history of races
> that are normally not caught without these enabled. But I think we
> have come a long way from that and these can be removed now.
> 

Thank you both. Applied to linux-kselftest next for Linux 6.5-rc1
This gives us time to ensure the above mentioned races are no
longer an issue.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-06-09 18:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-06 14:11 [PATCH] selftests/cpufreq: Don't enable generic lock debugging options Mark Brown
2023-06-07  3:45 ` Viresh Kumar
2023-06-09 18:52   ` Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).