On Tue, Sep 15, 2015 at 11:15:29PM +0200, Arnd Bergmann wrote: > On Tuesday 15 September 2015 10:06:07 Peter Zijlstra wrote: > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html