Hi Steven, On 2/14/19 8:07 PM, Steven Rostedt wrote: > On Thu, 14 Feb 2019 19:57:56 +0100 > Lukasz Luba <[email protected]> wrote: > >> This patch add basic tracing of the devfreq workqueue and delayed work. >> It aims to capture changes of the polling intervals and device state. >> >> Reviewed-by: Chanwoo Choi <[email protected]> >> Signed-off-by: Lukasz Luba <[email protected]> >> --- >> drivers/devfreq/devfreq.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index 0ae3de7..c9714fd 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -29,6 +29,9 @@ >> #include <linux/of.h> >> #include "governor.h" >> >> +#define CREATE_TRACE_POINTS >> +#include <trace/events/devfreq.h> >> + >> static struct class *devfreq_class; >> >> /* >> @@ -394,6 +397,11 @@ static void devfreq_monitor(struct work_struct *work) >> queue_delayed_work(devfreq_wq, &devfreq->work, >> msecs_to_jiffies(devfreq->profile->polling_ms)); >> mutex_unlock(&devfreq->lock); >> + >> + trace_devfreq_monitor(dev_name(&devfreq->dev), devfreq->previous_freq, >> + devfreq->profile->polling_ms, >> + devfreq->last_status.busy_time, >> + devfreq->last_status.total_time); > > I would pass just devfreq into the tracepoint (it will be less work by > gcc in this code path), and do all this work in the TP__fast_assign() > > The string can still handle this with: > > __string(dev_name, dev_name(&devfreq->dev)) > > And assigned: > > __assign_str(dev_name, dev_name(&devfreq->dev)) > > But the rest of fast assign should be: > > TP_fast_assign( > __entry->freq = devfreq->previous_freq; > __entry->busy_time = devfreq->last_status.busy_time; > __entry->total_time = devfreq->last_status.total_time; > __entry->polling_ms = devfreq->profile->polling_ms; > __assign_str(dev_name, dev_name(&deqfreq->dev)); > } I agree, it makes sense. Let me test this approach and then send v3 patches. Thank you for the review.
Regards, Lukasz > > -- Steve > >> } >> >> /** > > >
