lkml.org 
[lkml]   [2012]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] power, trace: add tracing for device_resume
Date
On Saturday, May 19, 2012, Sameer Nanda wrote:
> On Fri, May 18, 2012 at 4:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, May 18, 2012 at 03:17:32PM -0700, Sameer Nanda wrote:
> >> On Fri, May 18, 2012 at 2:46 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Friday, May 18, 2012, Sameer Nanda wrote:
> >> >> On Fri, May 18, 2012 at 2:19 PM, Steven Rostedt <rostedt@goodmis.org> 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@goodmis.org> 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.
> >
> > You are kidding, right? You have control over your test systems, don't
> > bloat everyone's kernel by 5k just because your infrastructure is
> > somehow something that you feel you can't change.
>
> Fair enough. But having to modify the kernel command line to do this
> is clunky. How about exposing the ability to turn on these
> initcall_debug prints through a knob under /sys/power?

This might work, but first you'd need to make them depend on something
different from initcall_debug (and make that thing in turn be set if
initcall_debug is put into the kernel command line). Then, you could
export the new variable.
Greg, does that make sense to you?

Rafael


\
 
 \ /
  Last update: 2012-05-19 14:21    [from the cache]
©2003-2011 Jasper Spaans