lkml.org 
[lkml]   [2021]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 4/5] tracing: Unify the logic for function tracing options
From
Date
Hi Steven,

Hi Steven,

On 6.04.21 г. 1:15, Steven Rostedt wrote:
>>
>> @@ -235,30 +248,31 @@ static struct tracer function_trace;
>> static int
>> func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
>> {
>> - switch (bit) {
>> - case TRACE_FUNC_OPT_STACK:
>> - /* do nothing if already set */
>> - if (!!set == !!(func_flags.val & TRACE_FUNC_OPT_STACK))
>> - break;
>> + ftrace_func_t func;
>> + u32 new_flags_val;
> Nit, but the variable should just be "new_flags", which is consistent with
> old_flags. In the kernel we don't need to the variable names to be so
> verbose.
>
>>
>> - /* We can change this flag when not running. */
>> - if (tr->current_trace != &function_trace)
>> - break;
>> + /* Do nothing if already set. */
>> + if (!!set == !!(func_flags.val & bit))
>> + return 0;
>>
>> - unregister_ftrace_function(tr->ops);
>> + /* We can change this flag only when not running. */
>> + if (tr->current_trace != &function_trace)
>> + return 0;
>>
>> - if (set) {
>> - tr->ops->func = function_stack_trace_call;
>> - register_ftrace_function(tr->ops);
>> - } else {
>> - tr->ops->func = function_trace_call;
>> - register_ftrace_function(tr->ops);
>> - }
>> + new_flags_val = (func_flags.val & ~(1UL << (bit - 1)));
>> + new_flags_val |= (set << (bit - 1));
> bit is already the mask, no need to shift it, nor there's no reason for the
> extra set of parenthesis. And the above can be done in one line.
>
> new_flags = (func_flags.val & ~bit) | (set ? bit : 0);
>

OK, I totally misinterpreted the meaning of the "bit" argument of the
function. I did not realized it is a mask. I was thinking the argument
gives only the number of the bit that changes (like 5 for the 5-th bit
inside the mask).

Thanks!
Yordan


\
 
 \ /
  Last update: 2021-04-07 15:35    [W:0.073 / U:1.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site