Re: [PATCH V2 1/4] perf: Add hardware breakpoint address mask

[Jacob Shin]

On Tue, Apr 23, 2013 at 03:18:44PM +0200, Oleg Nesterov wrote:
> On 04/23, Jacob Shin wrote:
> >
> > +__weak int arch_validate_hwbkpt_addr_mask(struct perf_event *bp)
> > +{
> > +	return bp->attr.bp_addr_mask == 0;
> > +}
> > +
> >  static int validate_hw_breakpoint(struct perf_event *bp)
> >  {
> >  	int ret;
> > @@ -393,6 +398,10 @@ static int validate_hw_breakpoint(struct perf_event *bp)
> >  	if (ret)
> >  		return ret;
> >  
> > +	ret = arch_validate_hwbkpt_addr_mask(bp);
> > +	if (ret)
> > +		return ret;
> 
> Well, this looks obviously wrong?
> 
> arch_validate_hwbkpt_addr_mask() fails if bp_addr_mask == 0? and returns
> "1" as the error code.
> 
> Either it should returns something like "bp_addr_mask ? -ENOTSUPP : 0"
> or the caller should do "if (!validate_hw_breakpoint()) return -ERR".

You are absolutely right. I have mixed up "does this arch support address
masks?" vs "is this a valid address mask?" So sorry about that.

Here is the corrected patch: