linux-kernel - Re: [PATCH] clk: Add tracepoints for hardware operations

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:	Mon, 02 Feb 2015 11:41:40 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Steven Rostedt <rostedt@...dmis.org>
CC:	Mike Turquette <mturquette@...aro.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: Add tracepoints for hardware operations

On 02/02/15 08:00, Steven Rostedt wrote:
> On Fri, 30 Jan 2015 16:16:11 -0800
> Stephen Boyd <sboyd@...eaurora.org> wrote:
>
>> It's useful to have tracepoints around operations that change the
>> hardware state so that we can debug clock hardware performance
>> and operations. Four basic types of events are supported: on/off
>> events for enable, disable, prepare, unprepare that only record
>> an event and a clock name, rate changing events for
>> clk_set_{min_,max_}rate{_range}(), phase changing events for
>> clk_set_phase() and parent changing events for clk_set_parent().
>>
>> Cc: Steven Rostedt <rostedt@...dmis.org>
> I don't see anything wrong with the implementation of the tracepoints.
> Now whether or not they are useful is up to the clk maintainer to
> decide.
>
>> Signed-off-by: Stephen Boyd <sboyd@...eaurora.org>
>> ---
>>  drivers/clk/clk.c          |  32 ++++++++
>>  include/trace/events/clk.h | 198 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 230 insertions(+)
>>  create mode 100644 include/trace/events/clk.h
>>
>
>   
>>  unlock_out:
>> @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk)
>>  
>>  	WARN_ON(clk->enable_count > 0);
>>  
>> +	trace_clk_unprepare(clk);
>> +
>>  	if (clk->ops->unprepare)
>>  		clk->ops->unprepare(clk->hw);
>>  
>> +	trace_clk_unprepare_complete(clk);
>>  	clk_core_unprepare(clk->parent);
> I guess you do not care about the clk_core_unprepare time.

Function trace will handle that?

>
>>  }
>>  
>> @@ -901,6 +911,8 @@ static int clk_core_prepare(struct clk_core *clk)
>>  		if (ret)
>>  			return ret;
>>  
>> +		trace_clk_prepare(clk);
>> +
>>  		if (clk->ops->prepare) {
>>  			ret = clk->ops->prepare(clk->hw);
>>  			if (ret) {
>> @@ -908,6 +920,8 @@ static int clk_core_prepare(struct clk_core *clk)
>>  				return ret;
>>  			}
>>  		}
>> +
>> +		trace_clk_prepare_complete(clk);
> I'm curious to why you do not put the tracepoint within the if
> statement, and only show the tracepoints if the clock prepare is
> actually called. Also, if you exit out with that return, will you tools
> be OK with seeing the clk_prepare but not the clk_prepare_complete?
>

Ah good point. I'll rework it so we always get the tracepoint around the
clk op.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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