[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54C06CBE.7080607@huawei.com>
Date: Thu, 22 Jan 2015 11:21:34 +0800
From: Wang Nan <wangnan0@...wei.com>
To: Jérémie Galarneau
<jeremie.galarneau@...icios.com>
CC: Jiri Olsa <jolsa@...hat.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Li Zefan <lizefan@...wei.com>, <linux-kernel@...r.kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH 1/2] perf: convert: fix duplicate field names and avoid
reserved keywords.
On 2015/1/22 11:14, Jérémie Galarneau wrote:
> On Wed, Jan 21, 2015 at 8:38 PM, Wang Nan <wangnan0@...wei.com> wrote:
>> On 2015/1/21 23:56, Jérémie Galarneau wrote:
>>> On Wed, Jan 21, 2015 at 9:11 AM, Jiri Olsa <jolsa@...hat.com> wrote:
>>>> On Wed, Jan 21, 2015 at 11:23:54AM +0800, Wang Nan wrote:
>>>>> Some parameters of syscall tracepoints named as 'nr', 'event', etc.
>>>>> When dealing with them, perf convert to ctf meets some problem:
>>>>>
>>>>> 1. If a parameter with name 'nr', it will duplicate syscall's
>>>>> common field 'nr'. One such syscall is io_submit().
>>>>>
>>>>> 2. If a parameter with name 'event', it is denied to be inserted
>>>>> because 'event' is a babeltrace keywork. One such syscall is
>>>>> epoll_ctl.
>>>>
>>>> hum, so this problem 2 is detectable only via bt_ctf_event_class_add_field function?
>>>>
>>>> how big is the blaklist?
>>>>
>>>
>>> The blacklist is defined by the CTF specification here [1].
>>>
>>> Jérémie
>>>
>>> [1] http://git.efficios.com/?p=ctf.git;a=blob;f=common-trace-format-specification.txt;h=abe4fb70fff7f17f6e8242f313fb74bff44cf89a;hb=HEAD#l1477
>>
>> Is there any possibility that the someone expand the list?
>>
>
> Good question. There is discussion around a v1.9 version of the CTF
> spec going on at the moment (which should not affect the Babeltrace
> API).
>
Since the blacklist is expanding, do you think babeltrace API should provide a mean to
notify users the reason about the failure, such as returning meanful error code instead
of -1, or exporting validate_identifier() to users?
> As far as I know, adding "__attribute__" has been discussed. CC'ing
> Mathieu Desnoyer who may have other extensions in mind.
>
> Jérémie
>
>>>
>>>> SNIP
>>>>
>>>>> +}
>>>>> +
>>>>> static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>>>> struct format_field *fields,
>>>>> struct bt_ctf_event_class *event_class)
>>>>> @@ -577,6 +609,9 @@ static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>>>> for (field = fields; field; field = field->next) {
>>>>> struct bt_ctf_field_type *type;
>>>>> unsigned long flags = field->flags;
>>>>> + struct bt_ctf_field_type *f = NULL;
>>>>> + char *name;
>>>>> + int dup = 1;
>>>>>
>>>>> pr2(" field '%s'\n", field->name);
>>>>>
>>>>> @@ -595,14 +630,36 @@ static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>>>> if (flags & FIELD_IS_ARRAY)
>>>>> type = bt_ctf_field_type_array_create(type, field->arraylen);
>>>>>
>>>>> - ret = bt_ctf_event_class_add_field(event_class, type,
>>>>> - field->name);
>>>>> + /* Check name duplication */
>>>>> + name = field->name;
>>>>
>>>> could you please put this in separated function like 'get_field_name(..)'
>>>> so we dont polute this function even more
>>>>
>>>> name == get_field_name(...)
>>>> if (!name)
>>>> error path
>>>>
>>>>
>>>> thanks,
>>>> jirka
>>>
>>>
>>>
>>
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists