lkml.org 
[lkml]   [2020]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables
From
Date
On 11/05/2020 12:01, Jiri Olsa wrote:
> On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:
>
> SNIP
>
>> + &sys_event_tables);
>> + }
>> +
>> print_events_table_prefix(eventsfp, tblname);
>> return 0;
>> }
>> @@ -1180,7 +1253,6 @@ int main(int argc, char *argv[])
>> } else if (rc < 0) {
>> /* Make build fail */
>> fclose(eventsfp);
>> - free_arch_std_events();
>> ret = 1;
>> goto out_free_mapfile;
>> } else if (rc) {
>> @@ -1206,27 +1278,31 @@ int main(int argc, char *argv[])
>> if (close_table)
>> print_events_table_suffix(eventsfp);
>>
>> - if (!mapfile) {
>> - pr_info("%s: No CPU->JSON mapping?\n", prog);
>> - goto empty_map;
>> + if (mapfile) {
>> + if (process_mapfile(eventsfp, mapfile)) {
>> + pr_err("%s: Error processing mapfile %s\n", prog,
>> + mapfile);
>> + /* Make build fail */
>> + fclose(eventsfp);
>> + ret = 1;
>> + }
>> + } else {
>> + pr_err("%s: No CPU->JSON mapping?\n", prog);
>
> shouldn't we jump to empty_map in here? there still needs to be a
> mapfile, right?

In theory we could only support sys events :)

But I'll now make this a (empty map) failure case. And I think that
another error case handling needs fixing in my patch.


As for this:

+ fprintf(outfp, "struct pmu_sys_events pmu_sys_event_tables[] = {");
>> +
>> + list_for_each_entry(sys_event_table, &sys_event_tables, list) {
>> + fprintf(outfp, "\n\t{\n\t\t.table = %s,\n\t},",
>> + sys_event_table->name);
>> + }
>> + fprintf(outfp, "\n\t{\n\t\t.table = 0\n\t},");
>
> this will add extra tabs:
>
> {
> .table = 0
> },
>
> while the rest of the file starts items without any indent
>

I'll ensure the indent is the same.

BTW, is there anything to be said for removing the empty map feature
(and always breaking the perf build instead)? I guess that it was just
an early feature for dealing with unstable JSONs.

Thanks,
john

>
> jirka
>
>> }
>>
>> - if (process_mapfile(eventsfp, mapfile)) {
>> - pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
>> - /* Make build fail */
>> + if (process_system_event_tables(eventsfp)) {
>> fclose(eventsfp);
>> - free_arch_std_events();
>> ret = 1;
>> }
>>
>> -
>> goto out_free_mapfile;
>>
>> empty_map:
>> fclose(eventsfp);
>> create_empty_mapping(output_file);
>> - free_arch_std_events();
>> out_free_mapfile:
>> + free_arch_std_events();
>> + free_sys_event_tables();
>> free(mapfile);
>> return ret;
>> }
>
> SNIP
>
> .
>

\
 
 \ /
  Last update: 2020-05-11 17:04    [W:0.175 / U:0.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site