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

[Christophe Leroy]

Le 25/08/2020 à 13:07, Ravi Bangoria a écrit :
> Hi Christophe,
>
> > > 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)

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