diff mbox

[09/13] Move bp_type_idx to kernel/event/hw_breakpoint.c

Message ID 1446579994-9937-10-git-send-email-palmer@dabbelt.com (mailing list archive)
State New, archived
Headers show

Commit Message

Palmer Dabbelt Nov. 3, 2015, 7:46 p.m. UTC 
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.

The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel).  Since
there's only one kernel user, it's been moved to that file.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/hw_breakpoint.h | 10 ----------
 kernel/events/hw_breakpoint.c      | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

kernel test robot Nov. 3, 2015, 9:28 p.m. UTC | #1 
Hi Palmer,

[auto build test ERROR on v4.3-rc7]
[also ERROR on: next-20151103]

url:    https://github.com/0day-ci/linux/commits/Palmer-Dabbelt/Remove-ifdef-CONFIG_64BIT-from-all-asm-generic-fcntl-h/20151104-035501
base:   https://github.com/0day-ci/linux Palmer-Dabbelt/Remove-ifdef-CONFIG_64BIT-from-all-asm-generic-fcntl-h/20151104-035501
config: powerpc-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/hw_breakpoint.c: In function 'hw_breakpoint_slots':
>> arch/powerpc/kernel/hw_breakpoint.c:49:14: error: 'TYPE_DATA' undeclared (first use in this function)
     if (type == TYPE_DATA)
                 ^
   arch/powerpc/kernel/hw_breakpoint.c:49:14: note: each undeclared identifier is reported only once for each function it appears in

vim +/TYPE_DATA +49 arch/powerpc/kernel/hw_breakpoint.c

5aae8a53 K.Prasad       2010-06-15  43  
5aae8a53 K.Prasad       2010-06-15  44  /*
d09ec738 Paul Mackerras 2010-06-29  45   * Returns total number of data or instruction breakpoints available.
d09ec738 Paul Mackerras 2010-06-29  46   */
d09ec738 Paul Mackerras 2010-06-29  47  int hw_breakpoint_slots(int type)
d09ec738 Paul Mackerras 2010-06-29  48  {
d09ec738 Paul Mackerras 2010-06-29 @49  	if (type == TYPE_DATA)
d09ec738 Paul Mackerras 2010-06-29  50  		return HBP_NUM;
d09ec738 Paul Mackerras 2010-06-29  51  	return 0;		/* no instruction breakpoints available */
d09ec738 Paul Mackerras 2010-06-29  52  }

:::::: The code at line 49 was first introduced by commit
:::::: d09ec7387184eba9e3030496f0451204090ff610 powerpc, hw_breakpoint: Tell generic code we have no instruction breakpoints

:::::: TO: Paul Mackerras <paulus@samba.org>
:::::: CC: Paul Mackerras <paulus@samba.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Nov. 3, 2015, 9:29 p.m. UTC | #2 
Hi Palmer,

[auto build test ERROR on v4.3-rc7]
[also ERROR on: next-20151103]

url:    https://github.com/0day-ci/linux/commits/Palmer-Dabbelt/Remove-ifdef-CONFIG_64BIT-from-all-asm-generic-fcntl-h/20151104-035501
base:   https://github.com/0day-ci/linux Palmer-Dabbelt/Remove-ifdef-CONFIG_64BIT-from-all-asm-generic-fcntl-h/20151104-035501
config: arm-socfpga_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/kernel/ptrace.c: In function 'ptrace_get_hbp_resource_info':
>> arch/arm/kernel/ptrace.c:440:33: error: 'TYPE_INST' undeclared (first use in this function)
     num_brps = hw_breakpoint_slots(TYPE_INST);
                                    ^
   arch/arm/kernel/ptrace.c:440:33: note: each undeclared identifier is reported only once for each function it appears in
>> arch/arm/kernel/ptrace.c:441:33: error: 'TYPE_DATA' undeclared (first use in this function)
     num_wrps = hw_breakpoint_slots(TYPE_DATA);
                                    ^
--
   arch/arm/kernel/hw_breakpoint.c: In function 'hw_breakpoint_slots':
>> arch/arm/kernel/hw_breakpoint.c:290:7: error: 'TYPE_INST' undeclared (first use in this function)
     case TYPE_INST:
          ^
   arch/arm/kernel/hw_breakpoint.c:290:7: note: each undeclared identifier is reported only once for each function it appears in
>> arch/arm/kernel/hw_breakpoint.c:292:7: error: 'TYPE_DATA' undeclared (first use in this function)
     case TYPE_DATA:
          ^

vim +/TYPE_INST +440 arch/arm/kernel/ptrace.c

864232fa Will Deacon 2010-09-03  434  
864232fa Will Deacon 2010-09-03  435  static u32 ptrace_get_hbp_resource_info(void)
864232fa Will Deacon 2010-09-03  436  {
864232fa Will Deacon 2010-09-03  437  	u8 num_brps, num_wrps, debug_arch, wp_len;
864232fa Will Deacon 2010-09-03  438  	u32 reg = 0;
864232fa Will Deacon 2010-09-03  439  
864232fa Will Deacon 2010-09-03 @440  	num_brps	= hw_breakpoint_slots(TYPE_INST);
864232fa Will Deacon 2010-09-03 @441  	num_wrps	= hw_breakpoint_slots(TYPE_DATA);
864232fa Will Deacon 2010-09-03  442  	debug_arch	= arch_get_debug_arch();
864232fa Will Deacon 2010-09-03  443  	wp_len		= arch_get_max_wp_len();
864232fa Will Deacon 2010-09-03  444  

:::::: The code at line 440 was first introduced by commit
:::::: 864232fa1a2f8dfe003438ef0851a56722740f3e ARM: 6357/1: hw-breakpoint: add new ptrace requests for hw-breakpoint interaction

:::::: TO: Will Deacon <will.deacon@arm.com>
:::::: CC: Russell King <rmk+kernel@arm.linux.org.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Peter Zijlstra Nov. 4, 2015, 11:41 a.m. UTC | #3 
On Tue, Nov 03, 2015 at 11:46:30AM -0800, Palmer Dabbelt wrote:
> This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
> 
> The names in here are so generic that I don't think it's a good idea
> to expose them to userspace (or even the rest of the kernel).  Since
> there's only one kernel user, it's been moved to that file.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
> Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>

Assuming you want to keep all these patches together in a tree or so.
Let me know if you want me to take this patch.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Zijlstra Nov. 4, 2015, 12:21 p.m. UTC | #4 
On Wed, Nov 04, 2015 at 12:41:06PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 03, 2015 at 11:46:30AM -0800, Palmer Dabbelt wrote:
> > This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
> > 
> > The names in here are so generic that I don't think it's a good idea
> > to expose them to userspace (or even the rest of the kernel).  Since
> > there's only one kernel user, it's been moved to that file.
> > 
> > Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> > Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
> > Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
> 
> Assuming you want to keep all these patches together in a tree or so.
> Let me know if you want me to take this patch.
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Ah, build-bot finds your change is broken and you didn't grep right. It
is used in more places.

How about moving it to include/linux/hw_breakpoint.h, instead of having
it in the uapi crud?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Palmer Dabbelt Nov. 7, 2015, 6:44 a.m. UTC | #5 
On Wed, 04 Nov 2015 04:21:51 PST (-0800), peterz@infradead.org wrote:
> On Wed, Nov 04, 2015 at 12:41:06PM +0100, Peter Zijlstra wrote:
>> On Tue, Nov 03, 2015 at 11:46:30AM -0800, Palmer Dabbelt wrote:
>> > This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
>> >
>> > The names in here are so generic that I don't think it's a good idea
>> > to expose them to userspace (or even the rest of the kernel).  Since
>> > there's only one kernel user, it's been moved to that file.
>> >
>> > Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
>> > Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
>> > Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
>>
>> Assuming you want to keep all these patches together in a tree or so.
>> Let me know if you want me to take this patch.

Well, the plan was to try to get the whole patch set all upstream together.
I'd prefer that, because it'll be easier to make sure everything gets in before
the last checker patch.  Since it touches so many different places I'd be OK
with doing it peicemeal.

I'm kind of new to this whole process: do you think someone is likely to take
this patch set all together?

>> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Ah, build-bot finds your change is broken and you didn't grep right. It
> is used in more places.

Sorry about that, I must have mis-grep'd.  I guess that's what the build-bot is
for :).

> How about moving it to include/linux/hw_breakpoint.h, instead of having
> it in the uapi crud?

Yep, that makes sense.

I'm going to re-submit a v5 of this patch set, since there was also a missing
patch for blackfin that the buildbot found.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h
index b04000a..7a6a5a7 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
-};
-
 #endif /* _UAPI_LINUX_HW_BREAKPOINT_H */
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 92ce5f4..5ad117e 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -58,6 +58,16 @@  struct bp_cpuinfo {
 	unsigned int	flexible; /* XXX: placeholder, see fetch_this_slot() */
 };
 
+enum bp_type_idx {
+	TYPE_INST	= 0,
+#if defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
+	TYPE_DATA	= 0,
+#else
+	TYPE_DATA	= 1,
+#endif
+	TYPE_MAX
+};
+
 static DEFINE_PER_CPU(struct bp_cpuinfo, bp_cpuinfo[TYPE_MAX]);
 static int nr_slots[TYPE_MAX];