Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Andi Kleen]

All changes look good to me. Thanks for picking that up.

Reviewed-by: Andi Kleen <ak@linux.intel.com>

-- 
ak@linux.intel.com -- Speaking for myself only

------------------------------------------------------------------------------
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

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Ingo Molnar]

* Josh Triplett <josh@joshtriplett.org> wrote:

> This patch series makes it possible to disable HW_BREAKPOINTS, 
> PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, and ANON_INODES.  
> Without this patch series, all of these config options get 
> automatically selected on x86, making them impossible to disable.
>
> This is a revival of a previous patch series from Andi Kleen sent in 
> October 2013. [...]

So my main problem with those patches was that if HW_BREAKPOINTS is 
disabled then GDB 'hbreak' isn't simply disabled but fails in various 
non-obvious ways, taking down the rest of GDB with it, so it's in 
essence an ABI and tool breaker which is not very useful.

If all of ptrace is disabled then it perhaps behaves more cleanly, but 
we didn't allow the disabling of ptrace in the past, and I'm not sure 
we should start with it today.

So it's still a NAK for the time being.

Thanks,

	Ingo

------------------------------------------------------------------------------
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

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Josh Triplett]

On Mon, Mar 10, 2014 at 08:43:54AM +0100, Ingo Molnar wrote:
> 
> * Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > This patch series makes it possible to disable HW_BREAKPOINTS, 
> > PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, and ANON_INODES.  
> > Without this patch series, all of these config options get 
> > automatically selected on x86, making them impossible to disable.
> >
> > This is a revival of a previous patch series from Andi Kleen sent in 
> > October 2013. [...]
> 
> So my main problem with those patches was that if HW_BREAKPOINTS is 
> disabled then GDB 'hbreak' isn't simply disabled but fails in various 
> non-obvious ways, taking down the rest of GDB with it, so it's in 
> essence an ABI and tool breaker which is not very useful.

It's hidden behind EXPERT for a reason, and non-hardware breakpoints
still work just fine.  This is the kind of thing enabled on an embedded
system, where you're not going to be running GDB at all, let alone using
"hbreak".  Given that other options depending on EXPERT let you disable
things as critical as futexes or ELF binary support...

Would it help to have some more detailed help text for HW_BREAKPOINTS,
explaining that gdb's "hbreak" and similar will utterly fail if this
option is disabled?

Alternatively, would it suffice to make ptrace_set_debugreg always
return an error when !CONFIG_HW_BREAKPOINTS?  Would that produce error
behavior you'd prefer?

> If all of ptrace is disabled then it perhaps behaves more cleanly,

That's on my list for the future. :)

- Josh Triplett

------------------------------------------------------------------------------
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

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Ingo Molnar]

* Josh Triplett <josh@joshtriplett.org> wrote:

> On Mon, Mar 10, 2014 at 08:43:54AM +0100, Ingo Molnar wrote:
> > 
> > * Josh Triplett <josh@joshtriplett.org> wrote:
> > 
> > > This patch series makes it possible to disable HW_BREAKPOINTS, 
> > > PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, and ANON_INODES.  
> > > Without this patch series, all of these config options get 
> > > automatically selected on x86, making them impossible to 
> > > disable.
> > >
> > > This is a revival of a previous patch series from Andi Kleen 
> > > sent in October 2013. [...]
> > 
> > So my main problem with those patches was that if HW_BREAKPOINTS 
> > is disabled then GDB 'hbreak' isn't simply disabled but fails in 
> > various non-obvious ways, taking down the rest of GDB with it, so 
> > it's in essence an ABI and tool breaker which is not very useful.
> 
> It's hidden behind EXPERT for a reason, [...]

With many distros enabling CONFIG_EXPERT=y that's a distinction 
without much meaning.

> [...] and non-hardware breakpoints still work just fine. [...]

A lot of other stuff will work just fine as well. Yet my point was 
that 'hbreak' breaks in ugly ways.

> [...] This is the kind of thing enabled on an embedded system, where 
> you're not going to be running GDB at all, let alone using "hbreak".

So that is why I suggested making ptrace configurable. Perhaps.

> Given that other options depending on EXPERT let you disable things 
> as critical as futexes or ELF binary support...

Both of which will break in pretty clear ways. The granularity of how 
we can disable ABIs largely depends on how well actual user-space code 
handles the failures, and what I'm saying here is that the disabling 
of hardware breakpoints is too finegrained.

Disabling ptrace for embedded OTOH makes sense and gives us even more 
savings and security advantage.

Thanks,

	Ingo

------------------------------------------------------------------------------
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

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Oleg Nesterov]

On 03/10, Ingo Molnar wrote:
>
> So my main problem with those patches was that if HW_BREAKPOINTS is
> disabled then GDB 'hbreak' isn't simply disabled but fails in various
> non-obvious ways,

but the kernel should return an error. And this can happen anyway, say,
reserve_bp_slot() can fail because all slots are already used by perf.

And iirc, gdb always switches to the single-stepping if hbreak fails.

Oleg.


------------------------------------------------------------------------------
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

Re: [PATCH v2 0/7] Allow disabling HW_BREAKPOINTS, PERF_EVENTS, INSTRUCTION_DECODER, IRQ_WORK, ANON_INODES

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 03/10, Ingo Molnar wrote:
> >
> > So my main problem with those patches was that if HW_BREAKPOINTS is
> > disabled then GDB 'hbreak' isn't simply disabled but fails in various
> > non-obvious ways,
> 
> but the kernel should return an error. And this can happen anyway, say,
> reserve_bp_slot() can fail because all slots are already used by perf.
> 
> And iirc, gdb always switches to the single-stepping if hbreak fails.

When the old patches were submitted I tried it with modern GDB and it 
wasn't a good experience.

Thanks,

	Ingo

------------------------------------------------------------------------------
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

Re: [PATCH v2 4/7] trace: Make UPROBES depend on PERF_EVENTS

[Oleg Nesterov]

On 03/09, Josh Triplett wrote:
>
> UPROBES need the perf events code,

Actually it doesn't. But yes, the kernel/events directory is only built
if CONFIG_PERF_EVENTS.

See http://marc.info/?t=139201796100001

> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 015f85a..8639819 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -424,6 +424,7 @@ config UPROBE_EVENT
>  	bool "Enable uprobes-based dynamic events"
>  	depends on ARCH_SUPPORTS_UPROBES
>  	depends on MMU
> +	depends on PERF_EVENTS

Can't we delay this change? This conflicts with

	[PATCH v7 01/15] uprobes: Kconfig dependency fix
	http://marc.info/?l=linux-kernel&m=139422332915701

We need to untangle UPROBES and PERF, we can change the Makefile's or
move uprobe.c to kernel/.

Oleg.


------------------------------------------------------------------------------
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

Re: [PATCH v2 4/7] trace: Make UPROBES depend on PERF_EVENTS

[Josh Triplett]

On Mon, Mar 10, 2014 at 03:21:29PM +0100, Oleg Nesterov wrote:
> On 03/09, Josh Triplett wrote:
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index 015f85a..8639819 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -424,6 +424,7 @@ config UPROBE_EVENT
> >  	bool "Enable uprobes-based dynamic events"
> >  	depends on ARCH_SUPPORTS_UPROBES
> >  	depends on MMU
> > +	depends on PERF_EVENTS
> 
> Can't we delay this change? This conflicts with
> 
> 	[PATCH v7 01/15] uprobes: Kconfig dependency fix
> 	http://marc.info/?l=linux-kernel&m=139422332915701

That doesn't conflict; they both make exactly the same change, and they
should trivially merge.

- Josh Triplett

------------------------------------------------------------------------------
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

Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Oleg Nesterov]

On 03/09, Josh Triplett wrote:
>
> +#ifdef CONFIG_HW_BREAKPOINTS
> +
>  static void ptrace_triggered(struct perf_event *bp,
>  			     struct perf_sample_data *data,
>  			     struct pt_regs *regs)
> @@ -778,6 +780,34 @@ static int ptrace_set_debugreg(struct task_struct *tsk, int n,
>  	return rc;
>  }
>
> +#else
> +
> +/* Without breakpoints only handle DR6 */
> +static unsigned long ptrace_get_debugreg(struct task_struct *tsk, int n)
> +{
> +	struct thread_struct *thread = &tsk->thread;
> +
> +	if (n == 6)
> +		return thread->debugreg6;
> +	return 0;
> +}
> +
> +/* Without breakpoints only handle DR6 */
> +static int ptrace_set_debugreg(struct task_struct *tsk, int n,
> +			       unsigned long val)
> +{
> +	struct thread_struct *thread = &tsk->thread;
> +	int rc = -EIO;
> +
> +	if (n == 6) {
> +		thread->debugreg6 = val;
> +		rc = 0;
> +	}
> +	return rc;
> +}
> +
> +#endif

OK, but then perhaps this patch should also make thread_struct->ptrace_bps[]
depend on CONFIG_HW_BREAKPOINTS ?

This needs more changes though. In particular, it seems it makes sense to
move flush_ptrace_hw_breakpoint() to x86/kernel/ptrace.c, so that it can
live in the same ifdef'ed section.

copy_threads() needs a cleanup anyway...

But I won't insist if you prefer to do this later.

Oleg.


------------------------------------------------------------------------------
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

Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Oleg Nesterov]

On 03/10, Oleg Nesterov wrote:
>
> On 03/09, Josh Triplett wrote:
> >
> > +static unsigned long ptrace_get_debugreg(struct task_struct *tsk, int n)
> > +{
> > +	struct thread_struct *thread = &tsk->thread;
> > +
> > +	if (n == 6)
> > +		return thread->debugreg6;
> > +	return 0;
> > +}
> > +
> > +/* Without breakpoints only handle DR6 */
> > +static int ptrace_set_debugreg(struct task_struct *tsk, int n,
> > +			       unsigned long val)
> > +{
> > +	struct thread_struct *thread = &tsk->thread;
> > +	int rc = -EIO;
> > +
> > +	if (n == 6) {
> > +		thread->debugreg6 = val;
> > +		rc = 0;
> > +	}
> > +	return rc;
> > +}
> > +
> > +#endif
>
> OK, but then perhaps this patch should also make thread_struct->ptrace_bps[]
> depend on CONFIG_HW_BREAKPOINTS ?

and ->ptrace_dr7 at least.

And in fact, why do we need ptrace_get/set_debugreg(DR6) without HW_BREAKPOINTS?

Perhaps get/set should just return the error?

> But I won't insist if you prefer to do this later.

Yes.

Oleg.


------------------------------------------------------------------------------
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

Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Josh Triplett]

On Mon, Mar 10, 2014 at 03:42:55PM +0100, Oleg Nesterov wrote:
> On 03/10, Oleg Nesterov wrote:
> >
> > On 03/09, Josh Triplett wrote:
> > >
> > > +static unsigned long ptrace_get_debugreg(struct task_struct *tsk, int n)
> > > +{
> > > +	struct thread_struct *thread = &tsk->thread;
> > > +
> > > +	if (n == 6)
> > > +		return thread->debugreg6;
> > > +	return 0;
> > > +}
> > > +
> > > +/* Without breakpoints only handle DR6 */
> > > +static int ptrace_set_debugreg(struct task_struct *tsk, int n,
> > > +			       unsigned long val)
> > > +{
> > > +	struct thread_struct *thread = &tsk->thread;
> > > +	int rc = -EIO;
> > > +
> > > +	if (n == 6) {
> > > +		thread->debugreg6 = val;
> > > +		rc = 0;
> > > +	}
> > > +	return rc;
> > > +}
> > > +
> > > +#endif
> >
> > OK, but then perhaps this patch should also make thread_struct->ptrace_bps[]
> > depend on CONFIG_HW_BREAKPOINTS ?
> 
> and ->ptrace_dr7 at least.

Those do seem like nice optimizations to reduce the size of
thread_struct.

> And in fact, why do we need ptrace_get/set_debugreg(DR6) without HW_BREAKPOINTS?

Good question.  Andi?

> Perhaps get/set should just return the error?

Perhaps.

> > But I won't insist if you prefer to do this later.
> 
> Yes.

The optimization of thread_struct I would prefer to defer until after
this patch series, yes.

But if there's a better way to write ptrace_{g,s}et_debugreg when
!CONFIG_HW_BREAKPOINTS, I'll happily change that here.

- Josh Triplett

------------------------------------------------------------------------------
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

Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Andi Kleen]

> > And in fact, why do we need ptrace_get/set_debugreg(DR6) without HW_BREAKPOINTS?
> 
> Good question.  Andi?

dr6 controls single stepping which is useful even without hw/breakpoints.

Basically gdb will still work for 95+% of all use cases, everyone
not using watch points.

-Andi

------------------------------------------------------------------------------
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

Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Andi Kleen]

On Mon, Mar 10, 2014 at 10:41:06AM -0700, Andi Kleen wrote:
> > > And in fact, why do we need ptrace_get/set_debugreg(DR6) without HW_BREAKPOINTS?
> > 
> > Good question.  Andi?
> 
> dr6 controls single stepping which is useful even without hw/breakpoints.
> 
> Basically gdb will still work for 95+% of all use cases, everyone
> not using watch points.

Actually thinking about it more gdb should be using PTRACE_SINGLESTEP, not
direct access to DR6. So removing it is likely ok (but should verify)

-Andi

------------------------------------------------------------------------------
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

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

[Frederic Weisbecker]

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

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

[Josh Triplett]

On Mon, Mar 10, 2014 at 05:14:11PM +0100, Frederic Weisbecker wrote:
> 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.

It looks like hpa's concern was primarily that it should be possible to
have CONFIG_HW_BREAKPOINTS=y without CONFIG_PERF_EVENTS.  I agree, but
since right now you can't have *either* of them disabled, allowing both
to be disabled seems like an improvement.

> > +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

Fair enough.

- Josh Triplett

------------------------------------------------------------------------------
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

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

[Josh Triplett]

On Mon, Mar 10, 2014 at 05:14:11PM +0100, Frederic Weisbecker wrote:
> On Sun, Mar 09, 2014 at 08:12:53PM -0700, Josh Triplett wrote:
> > --- 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

So, the converse of that is that makefiles and code should never depend
on HAVE_* symbols; they should only be used in Kconfig as dependencies.
Right now, piles of things use HAVE_HW_BREAKPOINT to select things to
build.  So, the biggest chunk of this change is introducing a new dummy
symbol HW_BREAKPOINTS depending on HAVE_HW_BREAKPOINT and
HAVE_MIXED_BREAKPOINTS_REGS, and changing almost all the references to
HAVE_HW_BREAKPOINT to HW_BREAKPOINTS.

That's effectively patch 2/6 of your series.  Would you consider
reviving that patch, or should I pull that patch into this series
instead?

- Josh Triplett

------------------------------------------------------------------------------
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