Re: [PATCH v5 5/8] powerpc/watchpoint: Fix exception handling for CONFIG_HAVE_HW_BREAKPOINT=N

Le 25/08/2020 à 13:07, Ravi Bangoria a écrit :

Hi Christophe,

quoted

quoted

diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 57a0ab822334..866597b407bc 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c

@@ -286,11 +286,16 @@ long ppc_del_hwdebug(struct task_struct *child, long data)

      }
      return ret;
  #else /* CONFIG_HAVE_HW_BREAKPOINT */
+    if (child->thread.hw_brk[data - 1].flags & HW_BRK_FLAG_DISABLED)

I think child->thread.hw_brk[data - 1].flags & HW_BRK_FLAG_DISABLED should go around additionnal ()

Not sure I follow.

Neither do I ....

I thought that GCC would emit a warning for that, but in fact it only emit warnings for things like:

	if (flags & HW_BRK_FLAG_DISABLED == HW_BRK_FLAG_DISABLED)

quoted

quoted

+        goto del;
+
      if (child->thread.hw_brk[data - 1].address == 0)
          return -ENOENT;

What about replacing the above if by:
     if (!(child->thread.hw_brk[data - 1].flags) & HW_BRK_FLAG_DISABLED) &&
         child->thread.hw_brk[data - 1].address == 0)
         return -ENOENT;

okay.. that's more compact.

But more importantly, what I wanted to know is whether CONFIG_HAVE_HW_BREAKPOINT
is set or not in production/distro builds for 8xx. Because I see it's not set in
8xx defconfigs.

Yes in our production configs with have CONFIG_PERF_EVENTS, that implies CONFIG_HAVE_HW_BREAKPOINT

Christophe