From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938309AbeE1LaM (ORCPT ); Mon, 28 May 2018 07:30:12 -0400 Received: from ozlabs.org ([203.11.71.1]:39185 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938062AbeE1LaI (ORCPT ); Mon, 28 May 2018 07:30:08 -0400 Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au From: Michael Ellerman To: Frederic Weisbecker Cc: LKML , Jiri Olsa , Namhyung Kim , Joel Fernandes , Peter Zijlstra , Linus Torvalds , Yoshinori Sato , Benjamin Herrenschmidt , Catalin Marinas , Chris Zankel , Paul Mackerras , Thomas Gleixner , Will Deacon , Rich Felker , Ingo Molnar , Mark Rutland , Alexander Shishkin , Andy Lutomirski , Arnaldo Carvalho de Melo , Max Filippov Subject: Re: [PATCH 01/12] perf/breakpoint: Split attribute parse and commit In-Reply-To: <20180525135846.GC22082@lerouge> References: <1526697950-7091-1-git-send-email-frederic@kernel.org> <1526697950-7091-2-git-send-email-frederic@kernel.org> <87h8mxstou.fsf@concordia.ellerman.id.au> <20180525135846.GC22082@lerouge> Date: Mon, 28 May 2018 21:29:59 +1000 Message-ID: <87lgc43tmw.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Frederic Weisbecker writes: > On Thu, May 24, 2018 at 11:56:01AM +1000, Michael Ellerman wrote: >> Frederic Weisbecker writes: >> >> > diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c >> > index 6e28d28..51320c2 100644 >> > --- a/kernel/events/hw_breakpoint.c >> > +++ b/kernel/events/hw_breakpoint.c >> > @@ -424,19 +443,22 @@ static int validate_hw_breakpoint(struct perf_event *bp) >> > >> > int register_perf_hw_breakpoint(struct perf_event *bp) >> > { >> > - int ret; >> > + struct arch_hw_breakpoint hw; >> > + int err; >> > >> > - ret = reserve_bp_slot(bp); >> > - if (ret) >> > - return ret; >> > + err = reserve_bp_slot(bp); >> > + if (err) >> > + return err; >> > >> > - ret = validate_hw_breakpoint(bp); >> > - >> > - /* if arch_validate_hwbkpt_settings() fails then release bp slot */ >> > - if (ret) >> > + err = hw_breakpoint_parse(bp, &bp->attr, &hw); >> >> Is there a good reason we pass bp and bp->attr? (I assume so) >> >> That added to the confusion in the existing code I think. > > Yes, on breakpoint creation (which is the above function) it's not needed > but breakpoint modification wants it as we need to pass the attr that are > to be validated, and those are not yet copied to the breakpoint at this > stage. This happens in the end of the series. OK thanks. cheers