linux-kernel - Re: [PATCH 1/1] ftrace: fix dynamic ftrace filter reset issue

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]

Message-ID: <CA+jqyFC6ZNOaGBTPc7s7EoTsJBUn6Z+43H0At_YmQ7gsUiP4eA@mail.gmail.com>
Date:	Sat, 24 Mar 2012 10:23:42 +0530
From:	Rajesh Bhagat <rajesh.lnx@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] ftrace: fix dynamic ftrace filter reset issue

Hi Steven,

Just to add to it, This problem occurs only in case when same tracer
is enabled again i.e. function on function.
In other case code does not hit below code, and anyways
tracing_reset_online_cpus and trace->init is called to reset the
trace.

>>> -   if (t == current_trace)
>>> +   if (t == current_trace) {
>>> +       tracing_reset_online_cpus(tr);
>>>         goto out;
>>> +   }

My point is, this issue is specific to a situation i.e. same tracer
set again, hence making resetting the trace not a very obvious choice.

Please comment.

-Rajesh

On Sat, Mar 24, 2012 at 9:47 AM, Rajesh Bhagat <rajesh.lnx@...il.com> wrote:
> Hi Steven,
>
> Thanks for the reply.
>
> I was wondering, why would a user reset the trace __explicitly__
> without even using it.
> That is why i created a patch for it.
>
>>> /debug/tracing # echo function > current_tracer
>>> /debug/tracing # cat trace <------ Not performed
>>> /debug/tracing # echo schedule > set_ftrace_filter
>>> /debug/tracing # echo function > current_trace
>>> /debug/tracing # cat trace <------- Only this is performed
>
> - Rajesh
>
> On Sat, Mar 24, 2012 at 1:06 AM, Steven Rostedt <rostedt@...dmis.org> wrote:
>> On Wed, 2012-03-21 at 11:22 +0530, Rajesh Bhagat wrote:
>>> >From 60ec8a88ba671fa0b424f7b4467dba3ea8a2a2c8 Mon Sep 17 00:00:00 2001
>>> From: Rajesh Bhagat <rajesh.lnx@...il.com>
>>> Date: Tue, 20 Mar 2012 10:35:59 +0530
>>> Subject: [PATCH 1/1] ftrace: fix dynamic ftrace filter reset issue
>>>
>>> This patch resets the trace_array using tracing_reset_online_cpus, if same
>>> tracer is set again instead of just returning from funtion tracing_set_tracer.
>>>
>>> problem description
>>> -------------------
>>> once function tracer is set, set_ftrace_filter not working.
>>> /debug/tracing # echo function > current_tracer
>>> /debug/tracing # echo schedule > set_ftrace_filter
>>> /debug/tracing # echo function > current_tracer
>>
>> Sorry no. The correct way you want to do this is:
>>
>> /debug/tracing # echo > trace
>>
>> That will reset the buffer for you and give you the desired result.
>>
>> -- Steve
>>
>>
>>
>>> /debug/tracing #  cat trace
>>> # tracer: function
>>> #
>>> #           TASK-PID    CPU#    TIMESTAMP  FUNCTION
>>> #              | |       |          |         |
>>>           <idle>-0     [000]    21.997778: irq_enter <-handle_IRQ
>>>           <idle>-0     [000]    21.997785: idle_cpu <-irq_enter
>>>           <idle>-0     [000]    21.997794: local_bh_disable <-irq_enter
>>>           <idle>-0     [000]    21.997800: tick_check_idle <-irq_enter
>>>           <idle>-0     [000]    21.997807:
>>> tick_check_oneshot_broadcast <-tick_check_idle
>>>           <idle>-0     [000]    21.997814: _local_bh_enable <-irq_enter
>>>
>>> After applying this patch, only filtered functions are traced.
>>>
>>> Signed-off-by: Rajesh Bhagat <rajesh.lnx@...il.com>
>>> ---
>>>  kernel/trace/trace.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>>> index a3f1bc5..f4fb190 100644
>>> --- a/kernel/trace/trace.c
>>> +++ b/kernel/trace/trace.c
>>> @@ -3058,8 +3058,10 @@ static int tracing_set_tracer(const char *buf)
>>>         ret = -EINVAL;
>>>         goto out;
>>>     }
>>> -   if (t == current_trace)
>>> +   if (t == current_trace) {
>>> +       tracing_reset_online_cpus(tr);
>>>         goto out;
>>> +   }
>>>
>>>     trace_branch_disable();
>>>     if (current_trace && current_trace->reset)
>>
>>
--
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/