> On Jul 19, 2022, at 11:32 AM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Mon, 18 Jul 2022 16:59:51 +0000
> Song Liu <songliubraving@xxxxxx> wrote:
>
>>>> vim +/direct_mutex_locked +8197 kernel/trace/ftrace.c
>>>>
>>>> 8182
>>>> 8183 /**
>>>> 8184 * register_ftrace_function - register a function for profiling
>>>> 8185 * @ops: ops structure that holds the function for profiling.
>>>> 8186 *
>>>> 8187 * Register a function to be called by all functions in the
>>>> 8188 * kernel.
>>>> 8189 *
>>>> 8190 * Note: @ops->func and all the functions it calls must be labeled
>>>> 8191 * with "notrace", otherwise it will go into a
>>>> 8192 * recursive loop.
>>>> 8193 */
>>>> 8194 int register_ftrace_function(struct ftrace_ops *ops)
>>>> 8195 __releases(&direct_mutex)
>>>> 8196 {
>>>>> 8197 bool direct_mutex_locked = false;
>>>> 8198 int ret;
>>>> 8199
>>>> 8200 ftrace_ops_init(ops);
>>>> 8201
>>>> 8202 ret = prepare_direct_functions_for_ipmodify(ops);
>>>> 8203 if (ret < 0)
>>>> 8204 return ret;
>>>> 8205 else if (ret == 1)
>>>> 8206 direct_mutex_locked = true;
>>>
>>> Honestly, this is another horrible trick. Would it be possible to
>>> call prepare_direct_functions_for_ipmodify() with direct_mutex
>>> already taken?
>
> Agreed. I'm not sure why I didn't notice this in the other versions.
> Probably was looking too much at the other logic. :-/
>
>>>
>>> I mean something like:
>>>
>>> mutex_lock(&direct_mutex);
>>>
>>> ret = prepare_direct_functions_for_ipmodify(ops);
>>> if (ret)
>>> goto out:
>>>
>>> mutex_lock(&ftrace_lock);
>>> ret = ftrace_startup(ops, 0);
>>> mutex_unlock(&ftrace_lock);
>>>
>>> out:
>>> mutex_unlock(&direct_mutex);
>>> return ret;
>>
>> Yeah, we can actually do something like this. We can also move the
>> ops->flags & FTRACE_OPS_FL_IPMODIFY check to
>> register_ftrace_function(), so we only lock direct_mutex when when
>> it is necessary.
>
> No need. Just take the direct_mutex, and perhaps add a:
>
> lockdep_assert_held(&direct_mutex);
>
> in the prepare_direct_functions_for_ipmodify().
>
> This is far from a fast path to do any tricks in trying to optimize it.
Got it. I will fix these and send v5.
Thanks,
Song
