Messages in this thread |  | | Date | Mon, 31 Aug 2020 10:52:30 +0800 | From | Leo Yan <> | Subject | Re: [PATCH RESEND v1 02/11] perf mem: Introduce weak function perf_mem_events__ptr() |
| |
On Fri, Aug 28, 2020 at 04:40:29PM +0100, James Clark wrote: > Hi Leo, > > On 06/08/2020 04:07, Leo Yan wrote: > > > > for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) { > > - if (!perf_mem_events[j].record) > > + e = perf_mem_events__ptr(j); > > + if (!e->record) > > continue; > > > > - if (!perf_mem_events[j].supported) { > > + if (!e->supported) { > > pr_err("failed: event '%s' not supported\n", > > - perf_mem_events[j].name); > > + perf_mem_events__name(j)); > > free(rec_argv); > > return -1; > > Does it make sense to do something like: > > for(j = 0; e = perf_mem_events__ptr(j); j++) { > ... > } > > now that it's a weak function that returns NULL when the argument out of range. That way the caller > doesn't need to know about PERF_MEM_EVENTS__MAX as well and it could potentially be a different > value. I don't know if it would ever make sense to have a different number of events on different platforms?
Thanks for reviewing, James.
If you look into the later patch "perf mem: Support new memory event PERF_MEM_EVENTS__LOAD_STORE", you could find it introduces a new event which will be only used for Arm SPE but will not be used by other archs.
Your suggestion is good to encapsulate the macro PERF_MEM_EVENTS__MAX into perf_mem_events__ptr(), I will try it in next spin.
Thanks, Leo
|  |