lkml.org 
[lkml]   [2020]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] perf metric: Code cleanup with map_for_each_event()
From
Date
Hi Namhyung,

On 2020/9/17 13:19, Namhyung Kim wrote:
> Hello,
>
> On Thu, Sep 17, 2020 at 11:45 AM Wei Li <liwei391@huawei.com> wrote:
>>
>> Since we have introduced map_for_each_event() to walk the 'pmu_events_map',
>> clean up metricgroup__print() and metricgroup__has_metric() with it.
>>
>> Signed-off-by: Wei Li <liwei391@huawei.com>
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> A nit-pick below:
>
>
>> ---
>> tools/perf/util/metricgroup.c | 33 +++++++++++++--------------------
>> 1 file changed, 13 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 8831b964288f..3734cbb2c456 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -26,6 +26,17 @@
>> #include "util.h"
>> #include <asm/bug.h>
>>
>> +#define map_for_each_event(__pe, __idx, __map) \
>> + for (__idx = 0, __pe = &__map->table[__idx]; \
>> + __pe->name || __pe->metric_group || __pe->metric_name; \
>> + __pe = &__map->table[++__idx])
>> +
>> +#define map_for_each_metric(__pe, __idx, __map, __metric) \
>> + map_for_each_event(__pe, __idx, __map) \
>> + if (__pe->metric_expr && \
>> + (match_metric(__pe->metric_group, __metric) || \
>> + match_metric(__pe->metric_name, __metric)))
>> +
>
> You may consider adding a declaration of match_metric() here.
> Right now, all users are below the function so it's ok but
> having the macro here can enable future addition above IMHO.
>

Thanks for spotting that.
I'd like to move them just below match_metric() for now, when we really need
to use them somewhere else, we may move them to header file then.

Thanks,
Wei

\
 
 \ /
  Last update: 2020-09-17 13:57    [W:1.040 / U:0.516 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site