linux-kernel - Re: [PATCH v2] rtc: Add tracepoints for RTC system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Dec 2017 13:47:21 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Alexandre Belloni <alexandre.belloni@...e-electrons.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, linux-rtc@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v2] rtc: Add tracepoints for RTC system

Hi Alexandre,

On 13 December 2017 at 06:16, Alexandre Belloni
<alexandre.belloni@...e-electrons.com> wrote:
> Hi Baolin,
>
> On 16/11/2017 at 13:59:28 +0800, Baolin Wang wrote:
>> @@ -779,6 +797,7 @@ static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer)
>>       }
>>
>>       timerqueue_add(&rtc->timerqueue, &timer->node);
>> +     trace_rtc_timer_enqueue(timer);
>
> This doesn't apply because of 74717b28cb32e1ad3c1042cafd76b264c8c0f68d.
> Can you rebase?

Sure.

>
>> diff --git a/include/trace/events/rtc.h b/include/trace/events/rtc.h
>> new file mode 100644
>> index 0000000..b5a4add
>> --- /dev/null
>> +++ b/include/trace/events/rtc.h
>> @@ -0,0 +1,220 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM rtc
>> +
>> +#if !defined(_TRACE_RTC_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_RTC_H
>> +
>> +#include <linux/rtc.h>
>> +#include <linux/tracepoint.h>
>> +
>> +DECLARE_EVENT_CLASS(rtc_time_alarm_class,
>> +
>> +     TP_PROTO(struct rtc_time *tm, int err),
>> +
>> +     TP_ARGS(tm, err),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(int, sec)
>> +             __field(int, min)
>> +             __field(int, hour)
>> +             __field(int, mday)
>> +             __field(int, mon)
>> +             __field(int, year)
>> +             __field(time64_t, secs)
>> +             __field(int, err)
>> +     ),
>> +
>> +     TP_fast_assign(
>> +             __entry->sec = tm->tm_sec;
>> +             __entry->min = tm->tm_min;
>> +             __entry->hour = tm->tm_hour;
>> +             __entry->mday = tm->tm_mday;
>> +             __entry->mon = tm->tm_mon;
>> +             __entry->year = tm->tm_year;
>> +             __entry->secs = rtc_tm_to_time64(tm);
>> +             __entry->err = err;
>> +     ),
>> +
>> +     TP_printk("%d-%02d-%02d %02d:%02d:%02d UTC (%lld) (%d)",
>> +               __entry->year + 1900, __entry->mon + 1, __entry->mday,
>> +               __entry->hour, __entry->min, __entry->sec, __entry->secs,
>> +               __entry->err
>> +     )
>> +);
>> +
>
> Also, I'm a bit concerned about having a struct rtc_time here. I think
> its goal is mainly to have a nice representation on the time but maybe

Yes.

> the best would be to make printk able to pretty print the time (some
> patches were proposed).

If I understood your point correctly, you did not like the format of
TP_printk() here, right? So how about if I remove the 'struct
rtc_time' and just pass one 'ktime_t' parameter? But it will be not
readable for user to trace the RTC time/alarm.

>
> How bad would that be to change it later? I didn't follow the whole
> tracepoint ABI issue closely.


-- 
Baolin.wang
Best Regards