Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760250Ab2EVTTG (ORCPT ); Tue, 22 May 2012 15:19:06 -0400 Received: from mail-qa0-f49.google.com ([209.85.216.49]:34125 "EHLO mail-qa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755409Ab2EVTTD convert rfc822-to-8bit (ORCPT ); Tue, 22 May 2012 15:19:03 -0400 MIME-Version: 1.0 In-Reply-To: <201205192130.08573.rjw@sisk.pl> References: <1337365643-29261-1-git-send-email-snanda@chromium.org> <201205191359.20457.rjw@sisk.pl> <20120519163226.GA24612@kroah.com> <201205192130.08573.rjw@sisk.pl> From: Sameer Nanda Date: Tue, 22 May 2012 12:18:40 -0700 X-Google-Sender-Auth: wUf_Z6I2viFB4l3xWcmQZmQLMw0 Message-ID: Subject: Re: [PATCH] power, trace: add tracing for device_resume To: "Rafael J. Wysocki" Cc: Greg KH , Steven Rostedt , rob@landley.net, len.brown@intel.com, pavel@ucw.cz, fweisbec@gmail.com, mingo@redhat.com, jkosina@suse.cz, standby24x7@gmail.com, jj@chaosbits.net, paulmck@linux.vnet.ibm.com, josh@joshtriplett.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4390 Lines: 81 On Sat, May 19, 2012 at 12:30 PM, Rafael J. Wysocki wrote: > On Saturday, May 19, 2012, Greg KH wrote: >> On Sat, May 19, 2012 at 01:59:20PM +0200, Rafael J. Wysocki wrote: >> > On Saturday, May 19, 2012, Sameer Nanda wrote: >> > > On Fri, May 18, 2012 at 4:01 PM, Greg KH 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 wrote: >> > > >> > On Friday, May 18, 2012, Sameer Nanda wrote: >> > > >> >> On Fri, May 18, 2012 at 2:19 PM, Steven Rostedt wrote: >> > > >> >> > On Fri, 2012-05-18 at 13:58 -0700, Sameer Nanda wrote: >> > > >> >> >> On Fri, May 18, 2012 at 12:14 PM, Steven Rostedt 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? >> >> Maybe, I'd like to see a patch first before agreeing with it though :) > > Sure. New patch uploaded that uses the existing initcall_debug functionality. -- Sameer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/