Re: [PATCH] power, trace: add tracing for device_resume

[Sameer Nanda]

On Fri, May 18, 2012 at 2:46 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> On Friday, May 18, 2012, Sameer Nanda wrote:
>> On Fri, May 18, 2012 at 2:19 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>> > On Fri, 2012-05-18 at 13:58 -0700, Sameer Nanda wrote:
>> >> On Fri, May 18, 2012 at 12:14 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>> >> > On Fri, 2012-05-18 at 11:57 -0700, Sameer Nanda wrote:
>> >> >
>> >> >> AFAICT, they are used for something completely different -- help solve
>> >> >> suspend/resume issues by saving a hash in the RTC of the last device
>> >> >> that suspended/resumed. ÂThey don't use the perf tracing mechanism at
>> >> >> all.
>> >> >>
>> >> >
>> >> > Also note that all tracepoints have timestamps attached to them. You do
>> >> > not need to add deltas. Do that in the userspace tools that read the
>> >> > timestamps and events. This way you can have one DECLARE_EVENT_CLASS and
>> >> > three DEFINE_EVENTs. This will save space.
>> >>
>> >> Agreed on the space savings. ÂHowever, with the time_delta in the
>> >> trace message itself, a one line shell script [1] that sorts on the
>> >> time_delta field is sufficient to quickly spot the devices that take a
>> >> long time to resume. ÂWithout the time_delta field, the user tool is
>> >> more complex since it needs to first match up the device_resume_in,
>> >> device_resume_waited and device_resume_out traces and then calculate
>> >> time deltas.
>> >>
>> >> Seems like a worthwhile trade-off to me but I can take out the
>> >> time_delta if the general consensus is otherwise.
>> >
>> > Just note that every TRACE_EVENT() adds around 5k or more code. Every
>> > DEFINE_EVENT adds just about 300 bytes.
>>
>> Ok, let me respin the patch. ÂI am thinking of adding time_delta to
>> all three traces. ÂThat way we should get the space saving while still
>> allowing quick spotting of devices that take long time to resume.
>
> Well, what's wrong with the code in drivers/base/power/main.c that
> is activated by adding initcall_debug to the kernel command line?

Mostly that I hadn't looked closely at initcall_debug before writing my patch :)

Now that I have taken a look at it, the main issue is that the kernel
command line needs to be modified to activate it.  We cannot do this
for our automated regression suites since the kernel command line is
protected on Chrome OS systems.

Does it make sense to merge these tracepoints with initcall_debug
then?  The initcall_debug prints will still be conditional on the
kernel command line flag but the tracepoints wouldn't be.

Another possibility is to convert initcall_debug to use tracepoints
instead of printks.  But I am not sure if this would break any
existing tools that depend on initcall_debug on resume path?

>
> Rafael



-- 
Sameer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/