Re: [PATCH v3] time: alarmtimer: Add the trcepoints for alarmtimer

[Baolin Wang]

On 18 October 2016 at 03:03, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Tue, 11 Oct 2016 18:48:16 +0800
> Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>
>> ---
>> Changes since v2:
>>  - Save time as s64 type.
>>  - Remove 'process_name' parameter and add 'now' parameter.
>>  - Rename the trace event name.
>>  - Remove restart trace event.
>>  - Other optimization.
>> ---
>>  include/trace/events/alarmtimer.h |   92 +++++++++++++++++++++++++++++++++++++
>>  kernel/time/alarmtimer.c          |   16 ++++++-
>>  2 files changed, 106 insertions(+), 2 deletions(-)
>>  create mode 100644 include/trace/events/alarmtimer.h
>>
>> diff --git a/include/trace/events/alarmtimer.h b/include/trace/events/alarmtimer.h
>> new file mode 100644
>> index 0000000..4c92a3e
>> --- /dev/null
>> +++ b/include/trace/events/alarmtimer.h
>> @@ -0,0 +1,92 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM alarmtimer
>> +
>> +#if !defined(_TRACE_ALARMTIMER_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_ALARMTIMER_H
>> +
>> +#include <linux/alarmtimer.h>
>> +#include <linux/rtc.h>
>> +#include <linux/tracepoint.h>
>> +
>> +TRACE_DEFINE_ENUM(ALARM_REALTIME);
>> +TRACE_DEFINE_ENUM(ALARM_BOOTTIME);
>> +
>> +#define show_alarm_type(type)        __print_flags(type, " | ",      \
>> +     { 1 << ALARM_REALTIME, "ALARM_REALTIME" },              \
>> +     { 1 << ALARM_BOOTTIME, "ALARM_BOOTTIME" })
>
> Why have the "ALARM_" prefix in the string? We already know that these
> are "alarm" tracepoints. They just make the lines longer than they need
> to be.
>
> Other than that, from a tracing point of view, this looks fine to me.

Thanks Steven. I will remove the prefix and add your ACK in next version.

-- 
Baolin.wang
Best Regards