Re: [PATCH v2 6/7] x86: Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK

[Frederic Weisbecker <fweisbec@gmail.com>]

On Sun, Mar 09, 2014 at 08:12:53PM -0700, Josh Triplett wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Make HW_BREAKPOINTS a config option. HW_BREAKPOINTS depends on perf.
> This allows disabling PERF_EVENTS for systems that don't need it (e.g.
> anything not used for development)  This then allows disabling IRQ_WORK
> and INSTRUCTION_DECODER.
> 
> This change reduces the size of allnoconfig by 161k, a full 7.6%
> reduction:
>    text	   data	    bss	    dec	    hex	filename
>  788816	 131952	1214432	2135200	 2094a0	vmlinux-allnoconfig-with-perf
>  666361	  93020	1214368	1973749	 1e1df5	vmlinux-allnoconfig-no-perf
> 
> bloat-o-meter summary:
> add/remove: 1/1040 grow/shrink: 1/25 up/down: 82/-152214 (-152132)

My review hasn't changed much since last time (https://lkml.org/lkml/2013/10/4/483),
so I'm mostly copy pasting it here so that we can discuss if you want:

I'm personally not against the idea of allowing hw breakpoint support
to be disabled but hpa did not like much the idea. I must say he had
valid concerns though, see the thread of this patchset: https://lkml.org/lkml/2011/7/14/163

IMHO, a better solution would be to make the hw breakpoint subsystem work
without using perf.

I mean, perf should use the hw breakpoint, but hw breakpoint shouldn't use perf,
(in fact that's what we agreed on with hpa IIRC).
But right now there is a kind of circular dependency between both that makes
perf always built-in in x86. I'm all for solving that by making hw breakpoint independant
from perf, but I think Ingo had mixed feelings about doing it that way. And he had valid
concerns on that as well.

So we are a bit stuck :)

Also some comments below:

> 
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> [josh: Rebased; HW_BREAKPOINTS marked as EXPERT; commit message updated
>        with new statistics.]
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  arch/x86/Kconfig         | 12 +++++++++---
>  arch/x86/kernel/Makefile |  3 ++-
>  arch/x86/kvm/Kconfig     |  1 +
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0af5250..12e3386 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -70,9 +70,6 @@ config X86
>  	select HAVE_KERNEL_XZ
>  	select HAVE_KERNEL_LZO
>  	select HAVE_KERNEL_LZ4
> -	select HAVE_HW_BREAKPOINT
> -	select HAVE_MIXED_BREAKPOINTS_REGS
> -	select PERF_EVENTS
>  	select HAVE_PERF_EVENTS_NMI
>  	select HAVE_PERF_REGS
>  	select HAVE_PERF_USER_STACK_DUMP
> @@ -587,6 +584,15 @@ config SCHED_OMIT_FRAME_POINTER
>  
>  	  If in doubt, say "Y".
>  
> +config HW_BREAKPOINTS
> +	bool "Enable hardware breakpoints support" if EXPERT
> +	default y
> +	select HAVE_HW_BREAKPOINT
> +	select HAVE_MIXED_BREAKPOINTS_REGS
> +	---help---
> +	  Enable support for x86 hardware breakpoints for debuggers
> +	  and perf.  This will implicitly enable perf-events.
> +

HW_BREAKPOINTS must depend on HAVE_HW_BREAKPOINT, not the other way round.
HAVE_* Kconfig symbols must only express constant backend support, not a variable user choice...

ie, that's what I did in this patchset
https://lkml.org/lkml/2011/7/14/163

Thanks.

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech