Messages in this thread | | | Date | Fri, 4 Sep 2020 17:52:51 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 02/14] perf mem: Introduce weak function perf_mem_events__ptr() |
| |
On Fri, Sep 04, 2020 at 08:34:47AM +0800, Leo Yan wrote: > Hi Jiri, > > On Thu, Sep 03, 2020 at 03:50:54PM +0200, Jiri Olsa wrote: > > On Tue, Sep 01, 2020 at 09:38:03AM +0100, Leo Yan wrote: > > > > SNIP > > > > > @@ -2941,30 +2942,38 @@ static int perf_c2c__record(int argc, const char **argv) > > > rec_argv[i++] = "record"; > > > > > > if (!event_set) { > > > - perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true; > > > - perf_mem_events[PERF_MEM_EVENTS__STORE].record = true; > > > + e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD); > > > + e->record = true; > > > + > > > + e = perf_mem_events__ptr(PERF_MEM_EVENTS__STORE); > > > + e->record = true; > > > } > > > > > > - if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record) > > > + e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD); > > > + if (e->record) > > > rec_argv[i++] = "-W"; > > > > > > rec_argv[i++] = "-d"; > > > rec_argv[i++] = "--phys-data"; > > > rec_argv[i++] = "--sample-cpu"; > > > > > > - for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) { > > > - if (!perf_mem_events[j].record) > > > + j = 0; > > > + while ((e = perf_mem_events__ptr(j)) != NULL) { > > > + if (!e->record) { > > > > you could keep the above 'for loop' in here, it seems better > > than taking care of j++ > > Actually in patch v1 I did this way :) I followed James' suggestion to > encapsulate PERF_MEM_EVENTS__MAX into perf_mem_events__ptr(), thus > builtin-mem.c and buildin-c2c.c are not necessary to use > PERF_MEM_EVENTS__MAX in the loop and only needs to detect if the > pointer is NULL or not when return from perf_mem_events__ptr().
ah because u added that load_store event
> > How about change as below? > > for (j = 0; (e = perf_mem_events__ptr(j)) != NULL; j++) { > [...]
will this work? e will be NULL for first iteration no?
there are still other for loops with PERF_MEM_EVENTS__MAX used in the patch.. you overload the perf_mem_events access for arm, and add missing load_store NULL item to generic version, so there's always PERF_MEM_EVENTS__MAX items in the array
can we just use the current for loop and check for e->tag != NULL or any other field
thanks, jirka
| |