Re: [PATCH] ftrace: Replace ftrace_disabled variable with ftrace_is_dead function

[Bang Li]

On 2024/4/20 9:40, Steven Rostedt wrote:

On Fri, 19 Apr 2024 22:38:44 +0800
"Bang Li" <libang...@antgroup.com> wrote:

Use the existing function ftrace_is_dead to replace the variable to make
the code clearer.

Signed-off-by: Bang Li <libang...@antgroup.com>
---
  kernel/trace/ftrace.c | 46 +++++++++++++++++++++----------------------
  1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 50ca4d4f8840..4a08c79db677 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2693,7 +2693,7 @@ void __weak ftrace_replace_code(int mod_flags)
        int schedulable = mod_flags & FTRACE_MODIFY_MAY_SLEEP_FL;
        int failed;

- if (unlikely(ftrace_disabled))

+       if (unlikely(ftrace_is_dead()))
                return;

NACK!

ftrace_is_dead() is only there to make the static variable
"ftrace_disabled" available for code outside of ftrace.c. In ftrace.c,
it is perfectly fine to use ftrace_disabled.

-- Steve

Thank you for your explanation, let me understand this. How about
replacing ftrace_disabled with unlike(ftrace_disabled)?

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 50ca4d4f8840..76e0814723cb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6767,7 +6767,7 @@ void ftrace_release_mod(struct module *mod)

        mutex_lock(&ftrace_lock);

-       if (ftrace_disabled)
+       if (unlikely(ftrace_disabled))
                goto out_unlock;

        list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
@@ -6830,7 +6830,7 @@ void ftrace_module_enable(struct module *mod)

        mutex_lock(&ftrace_lock);

-       if (ftrace_disabled)
+       if (unlikely(ftrace_disabled))
                goto out_unlock;

        /*

Thanks again for the review!
Bang