From: Frederic Weisbecker <fweisbec@gmail.com>
To: Andi Kleen <andi@firstfloor.org>,
"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
peterz@infradead.org, Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 6/6] x86: Allow disabling HW_BREAKPOINTS and PERF_EVENTS
Date: Sat, 5 Oct 2013 00:27:36 +0200 [thread overview]
Message-ID: <20131004222731.GA28647@localhost.localdomain> (raw)
In-Reply-To: <1380922788-23112-7-git-send-email-andi@firstfloor.org>
On Fri, Oct 04, 2013 at 02:39:48PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> As suggested by Ingo.
>
> 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)
>
> Disabling PERF_EVENTS saves over 700k of kernel text (~5% of my config)
> and significant data/bss:
>
> text data bss dec hex filename
> 13692640 1922416 1478656 17093712 104d450 obj/vmlinux
> 12980092 1787544 1470464 16238100 f7c614 obj-noperf/vmlinux
>
> I didn't make it depend on CONFIG_EXPERT for now, as the system
> should be very usable even without it. To actually disable perf
> a couple of options depending on it need to be disabled, including
> KVM and HW_BREAKPOINTS.
>
> Longer term it would be probably nice to have modular perf.
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:
>
> Cc: fweisbec@gmail.com
> Cc: peterz@infradead.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> arch/x86/Kconfig | 11 ++++++++---
> arch/x86/kernel/Makefile | 3 ++-
> arch/x86/kvm/Kconfig | 1 +
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c9d2b81..a8418ed 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -67,9 +67,6 @@ config X86
> select HAVE_KERNEL_XZ
> select HAVE_KERNEL_LZO
> select HAVE_KERNEL_LZ4
> - select HAVE_HW_BREAKPOINT
> - select HAVE_MIXED_BREAKPOINTS_REGS
I really wish we stick the HAVE_*
Kconfig symbols to the simple role of expressing a
constant arch ability, not a variable user choice...
The user choice itself is CONFIG_HW_BREAKPOINT, the frontend,
which depends on the arch ability that is CONFIG_HAVE_HW_BREAKPOINT,
the backend.
ie, that's what I did in this patchset
https://lkml.org/lkml/2011/7/14/163
Thanks.
> - select PERF_EVENTS
> select HAVE_PERF_EVENTS_NMI
> select HAVE_PERF_REGS
> select HAVE_PERF_USER_STACK_DUMP
> @@ -602,6 +599,14 @@ config SCHED_OMIT_FRAME_POINTER
>
> If in doubt, say "Y".
>
> +config HW_BREAKPOINTS
> + bool "Enable hardware breakpoints support"
> + 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.
> +
> menuconfig HYPERVISOR_GUEST
> bool "Linux guest support"
> ---help---
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index a5408b9..1b09567 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -31,7 +31,7 @@ obj-$(CONFIG_X86_64) += vsyscall_64.o
> obj-$(CONFIG_X86_64) += vsyscall_emu_64.o
> obj-y += bootflag.o e820.o
> obj-y += pci-dma.o quirks.o topology.o kdebugfs.o
> -obj-y += alternative.o i8253.o pci-nommu.o hw_breakpoint.o
> +obj-y += alternative.o i8253.o pci-nommu.o
> obj-y += tsc.o io_delay.o rtc.o
> obj-y += pci-iommu_table.o
> obj-y += resource.o
> @@ -73,6 +73,7 @@ obj-$(CONFIG_DOUBLEFAULT) += doublefault.o
> obj-$(CONFIG_KGDB) += kgdb.o
> obj-$(CONFIG_VM86) += vm86_32.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> +obj-$(CONFIG_HW_BREAKPOINTS) += hw_breakpoint.o
>
> obj-$(CONFIG_HPET_TIMER) += hpet.o
> obj-$(CONFIG_APB_TIMER) += apb_timer.o
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index a47a3e5..de4190a 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -36,6 +36,7 @@ config KVM
> select TASKSTATS
> select TASK_DELAY_ACCT
> select PERF_EVENTS
> + select HW_BREAKPOINTS
> select HAVE_KVM_MSI
> select HAVE_KVM_CPU_RELAX_INTERCEPT
> ---help---
> --
> 1.8.3.1
>
next prev parent reply other threads:[~2013-10-04 22:27 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-04 21:39 Allow disabling perf on x86 Andi Kleen
2013-10-04 21:39 ` [PATCH 1/6] perf, x86, amd: Move __get_ibs_caps into common amd CPU file Andi Kleen
2013-10-04 21:39 ` [PATCH 2/6] perf, x86: Make perf amd ibs code depend on PERF_EVENTS and SUP_AMD Andi Kleen
2013-10-04 21:39 ` [PATCH 3/6] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace Andi Kleen
2013-10-04 21:39 ` [PATCH 4/6] trace: Make UPROBES depend on PERF_EVENTS Andi Kleen
2013-10-05 0:52 ` Steven Rostedt
2013-10-05 3:25 ` Andi Kleen
2013-10-04 21:39 ` [PATCH 5/6] x86, kgdb: Support compiling without hardware break points Andi Kleen
2013-10-04 21:39 ` [PATCH 6/6] x86: Allow disabling HW_BREAKPOINTS and PERF_EVENTS Andi Kleen
2013-10-04 22:27 ` Frederic Weisbecker [this message]
2013-10-05 7:08 ` Ingo Molnar
2013-10-05 17:05 ` Andi Kleen
2013-10-08 19:55 ` Frederic Weisbecker
2013-10-08 20:05 ` Peter Zijlstra
2013-10-08 20:34 ` Frederic Weisbecker
2013-10-08 20:35 ` Frederic Weisbecker
2013-10-09 6:02 ` Ingo Molnar
2013-10-06 16:49 ` Ingo Molnar
2013-10-08 6:59 ` Ingo Molnar
2013-10-08 15:35 ` Steven Rostedt
2013-10-08 19:39 ` Ingo Molnar
2013-10-08 15:42 ` Vince Weaver
2013-10-08 15:51 ` Dave Jones
2013-10-08 16:22 ` David Ahern
2013-10-08 16:24 ` Dave Jones
2013-10-08 19:36 ` Ingo Molnar
2013-10-08 20:19 ` Steven Rostedt
2013-10-09 3:21 ` Andi Kleen
2013-10-09 3:24 ` Andi Kleen
2013-10-09 3:39 ` Andi Kleen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131004222731.GA28647@localhost.localdomain \
--to=fweisbec@gmail.com \
--cc=ak@linux.intel.com \
--cc=andi@firstfloor.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).