Messages in this thread | | | Subject | Re: [PATCH v2 4/5] tracing: Unify the logic for function tracing options | From | "Yordan Karadzhov (VMware)" <> | Date | Wed, 7 Apr 2021 16:34:11 +0300 |
| |
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
| |