On Tue, Sep 15, 2015 at 11:15:29PM +0200, Arnd Bergmann wrote:

On Tuesday 15 September 2015 10:06:07 Peter Zijlstra wrote:

quoted

quoted

diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h
index b04000a2296a..7a6a5a7f9511 100644
--- a/include/uapi/linux/hw_breakpoint.h
+++ b/include/uapi/linux/hw_breakpoint.h

@@ -17,14 +17,4 @@ enum {
      HW_BREAKPOINT_INVALID   = HW_BREAKPOINT_RW | HW_BREAKPOINT_X,
 };
 
-enum bp_type_idx {
-     TYPE_INST       = 0,
-#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
-     TYPE_DATA       = 0,
-#else
-     TYPE_DATA       = 1,
-#endif
-     TYPE_MAX
-};

This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:

  perf_event_attr::bp_type

So removing them will break things.

Frederic?

If user space actually relies on the definition from this header file,
then it will use the wrong one on x86 and get 'TYPE_DATA = 1', while the
kernel uses 'TYPE_DATA = 0'.

That seems unlikely to work, so I suspect it gets a different definition.
If it uses this definition and it does work, we can probably use

#if defined(__KERNEL__) && defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)

but that requires a comment explaining exactly why that works.

I think this TYPE_DATA/TYPE_INST can be safely removed from uapi. This is only
about internal kernel code. Userspace only relies on HW_BREAKPOINT_[R/W/X]
to tell about the nature of the breakpoint.

If userspace ever relies on it, which I have no idea why, it even needs to define
CONFIG_HAVE_MIXED_BREAKPOINTS_REGS. No really there shouldn't be any user of that outside
the kernel.

Thanks.

	Arnd