Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-26 Thread Jiri Olsa
On Mon, Nov 13, 2017 at 12:02:56AM -0800, Milind Chabbi wrote: > SNIP > > On Sun, Nov 12, 2017 at 11:46 PM, Jiri Olsa wrote: > > > but you closed fd4 before openning fd5..? > > Yes, that is correct. I closed fd4. The reason is by closing fd4, we > are having a total of 3 hardware breakpoints ac

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-13 Thread Milind Chabbi
SNIP On Sun, Nov 12, 2017 at 11:46 PM, Jiri Olsa wrote: > but you closed fd4 before openning fd5..? Yes, that is correct. I closed fd4. The reason is by closing fd4, we are having a total of 3 hardware breakpoints active, but we are making the software counting in the kernel think that four TYP

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-12 Thread Jiri Olsa
On Sun, Nov 12, 2017 at 11:09:23AM -0800, Milind Chabbi wrote: > , > > On Thu, Nov 9, 2017 at 10:59 AM, Milind Chabbi > wrote: > > SNIP > > > > On Thu, Nov 9, 2017 at 5:12 AM, Jiri Olsa wrote: > >> > >> > >> how about something like below (untested) > >> > >> looks like there's no irq caller f

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-12 Thread Milind Chabbi
, On Thu, Nov 9, 2017 at 10:59 AM, Milind Chabbi wrote: > SNIP > > On Thu, Nov 9, 2017 at 5:12 AM, Jiri Olsa wrote: >> >> >> how about something like below (untested) >> >> looks like there's no irq caller for modify_user_hw_breakpoint, >> so we should be fine with locking nr_bp_mutex >> >> jir

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-09 Thread Milind Chabbi
SNIP On Thu, Nov 9, 2017 at 5:12 AM, Jiri Olsa wrote: > > > how about something like below (untested) > > looks like there's no irq caller for modify_user_hw_breakpoint, > so we should be fine with locking nr_bp_mutex > > jirka > > > --- > diff --git a/kernel/events/hw_breakpoint.c b/kernel/event

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-09 Thread Jiri Olsa
On Thu, Nov 09, 2017 at 08:46:58AM +0100, Jiri Olsa wrote: SNIP > > Jirka, > > > > I carefully looked at bp_cpuinfo[] and nr_slots[] data structures. > > nr_slots[] is an array of length two (one slot of TYPE_INST and > > another for TYPE_DATA). > > The accounting "thinks" that there is one limi

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Jiri Olsa
On Wed, Nov 08, 2017 at 08:59:22AM -0800, Milind Chabbi wrote: > On Wed, Nov 8, 2017 at 7:57 AM, Jiri Olsa wrote: > > On Wed, Nov 08, 2017 at 07:51:10AM -0800, Milind Chabbi wrote: > >> On Wed, Nov 8, 2017 at 7:12 AM, Jiri Olsa wrote: > >> > >> > > I am not able to fully understand your concern.

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Milind Chabbi
On Wed, Nov 8, 2017 at 7:57 AM, Jiri Olsa wrote: > On Wed, Nov 08, 2017 at 07:51:10AM -0800, Milind Chabbi wrote: >> On Wed, Nov 8, 2017 at 7:12 AM, Jiri Olsa wrote: >> >> > > I am not able to fully understand your concern. >> > > Can you point to a code file and line related to your observation?

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Jiri Olsa
On Wed, Nov 08, 2017 at 07:51:10AM -0800, Milind Chabbi wrote: > On Wed, Nov 8, 2017 at 7:12 AM, Jiri Olsa wrote: > > > > I am not able to fully understand your concern. > > > Can you point to a code file and line related to your observation? > > > The patch is modeled after the existing modify_u

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Milind Chabbi
On Wed, Nov 8, 2017 at 7:12 AM, Jiri Olsa wrote: > > I am not able to fully understand your concern. > > Can you point to a code file and line related to your observation? > > The patch is modeled after the existing modify_user_hw_breakpoint() function > > present in events/hw_breakpoint.c; don't

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Jiri Olsa
On Wed, Nov 08, 2017 at 07:02:22AM -0800, Milind Chabbi wrote: > On Wed, Nov 8, 2017 at 6:15 AM, Jiri Olsa wrote: > > On Mon, Nov 06, 2017 at 07:04:40AM -0800, Milind Chabbi wrote: > >> Hi Jirka, > >> > >> I see the tabs in my sent email, do you have suggestions on how best to > >> send this patch

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Milind Chabbi
On Wed, Nov 8, 2017 at 6:15 AM, Jiri Olsa wrote: > On Mon, Nov 06, 2017 at 07:04:40AM -0800, Milind Chabbi wrote: >> Hi Jirka, >> >> I see the tabs in my sent email, do you have suggestions on how best to >> send this patch so that the tabs are preserved by the email client? >> Can anybody else al

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-08 Thread Jiri Olsa
On Mon, Nov 06, 2017 at 07:04:40AM -0800, Milind Chabbi wrote: > Hi Jirka, > > I see the tabs in my sent email, do you have suggestions on how best to > send this patch so that the tabs are preserved by the email client? > Can anybody else also check if they received with/without tabs? > > releas

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-08 Thread kbuild test robot
Hi Milind, Thank you for the patch! Yet something to improve: [auto build test ERROR on tip/perf/core] [also build test ERROR on v4.14-rc8 next-20171108] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-08 Thread kbuild test robot
Hi Milind, Thank you for the patch! Yet something to improve: [auto build test ERROR on tip/perf/core] [also build test ERROR on v4.14-rc8 next-20171108] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Milind Chabbi
On Tue, Nov 7, 2017 at 11:01 AM, Peter Zijlstra wrote: > On Tue, Nov 07, 2017 at 09:42:25AM -0800, Milind Chabbi wrote: >> I am fine with Andi's suggestion. In summary, > > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > A: Top-post

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Peter Zijlstra
On Tue, Nov 07, 2017 at 09:42:25AM -0800, Milind Chabbi wrote: > I am fine with Andi's suggestion. In summary, A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail?

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Milind Chabbi
I am fine with Andi's suggestion. In summary, 1. I will introduce an ioctl flag _IOC_MODIFY_ATTRIBUTES. (Yes, plural ATTRIBUTES not ATTRIBUTE) 2. Currently, implement only updates to breakpoints and all others will fail with -EOPNOTSUPP. 3. The implementation of breakpoint update shall check the

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Andi Kleen
On Tue, Nov 07, 2017 at 07:43:35AM -0800, Milind Chabbi wrote: > Peter, > > Generic update perf_event_attr interface is noble but impractical. > It will cause a validation nightmare. > Many of the behaviors or choices will become hard to reason. I don't think you would necessarily need to support

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Andi Kleen
On Tue, Nov 07, 2017 at 09:15:41AM +0100, Peter Zijlstra wrote: > On Mon, Nov 06, 2017 at 03:16:58PM -0800, Andi Kleen wrote: > > > +static int _perf_event_modify_breakpoint(struct perf_event *bp, > > > + struct perf_event_attr *attr) > > > +{ > > > + u64 old_addr =

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Milind Chabbi
Peter, Generic update perf_event_attr interface is noble but impractical. It will cause a validation nightmare. Many of the behaviors or choices will become hard to reason. If somebody changes "sample_period" it is unclear what to do it the number of samples collected so far exceeds the newly con

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Peter Zijlstra
On Mon, Nov 06, 2017 at 05:09:15PM -0500, Milind Chabbi wrote: > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > index 362493a..d458214 100644 > --- a/include/uapi/linux/perf_event.h > +++ b/include/uapi/linux/perf_event.h > @@ -433,6 +433,8 @@ struct perf_event_att

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-07 Thread Peter Zijlstra
On Mon, Nov 06, 2017 at 03:16:58PM -0800, Andi Kleen wrote: > > +static int _perf_event_modify_breakpoint(struct perf_event *bp, > > +struct perf_event_attr *attr) > > +{ > > + u64 old_addr = bp->attr.bp_addr; > > + u64 old_len = bp->attr.bp_len; > > + int

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-06 Thread Andi Kleen
> +static int _perf_event_modify_breakpoint(struct perf_event *bp, > + struct perf_event_attr *attr) > +{ > + u64 old_addr = bp->attr.bp_addr; > + u64 old_len = bp->attr.bp_len; > + int old_type = bp->attr.bp_type; > + int err = 0; > + > + _p

[PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

2017-11-06 Thread Milind Chabbi
Problem and motivation: Once a breakpoint perf event (PERF_TYPE_BREAKPOINT) is created, there is no flexibility to change the breakpoint type (bp_type), breakpoint address (bp_addr), or breakpoint length (bp_len). The only option is to close the perf event and configure a new breakpoint event. This

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-06 Thread Arnaldo Carvalho de Melo
Em Mon, Nov 06, 2017 at 07:00:29AM -0800, Milind Chabbi escreveu: > Hi Jirka, > > I see the tabs in my sent email, do you have suggestions on how best to > send this patch so that the tabs are preserved by the email client? Documentation/process/email-clients.rst - Arnaldo > Can anybody else a

Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT

2017-11-06 Thread Jiri Olsa
On Sun, Nov 05, 2017 at 02:35:34PM -0800, Milind Chabbi wrote: SNIP > +static int _perf_event_modify_breakpoint(struct perf_event *bp, > + struct perf_event_attr *attr) > +{ > + u64 old_addr = bp->attr.bp_addr; > + u64 old_len = bp->attr.bp_len; > + int old_type = bp->attr.bp_type; > + int err =