kexec jump: __ftrace_enabled_save/restore
Subsystems (3)
PATCH 6/7 kexec jump: __ftrace_enabled_save/restore
Add __ftrace_enabled_save/restore, used to disable ftrace for a while. Now, this is used by kexec jump, which need a version without lock, for general situation, a locked version should be used.
--- include/linux/ftrace.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
include/linux/ftrace.h +21 -0
@@ -98,6 +98,27 @@ static inline void tracer_disable(void)#endif } +/* Ftrace disable/restore without lock. Some synchronization mechanism +* must be used to prevent ftrace_enabled to be changed between +* disable/restore. */ +static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else +return 0 ; +#endif +} + +static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +} +#ifdef CONFIG_FRAME_POINTER /* TODO: need to fix this for ARM */ # define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 30 lines · Huang Ying
Add __ftrace_enabled_save/restore, used to disable ftrace for a while. Now, this is used by kexec jump, which need a version without lock, for general situation, a locked version should be used.
Signed-off-by Huang Ying <[email protected]>--- include/linux/ftrace.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)include/linux/ftrace.h +14
-0@@ -98,6 +98,27 @@ static inline void tracer_disable(void)#endif } +/* Ftrace disable/restore without lock. Some synchronization mechanism +* must be used to prevent ftrace_enabled to be changed between +* disable/restore. */ +static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else +return 0 ; +#endif +} +
include/linux/ftrace.h static inline void tracer_disable(void) · line 124
Quoted 10 lines
include/linux/ftrace.h +7
-0+static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +} +#ifdef CONFIG_FRAME_POINTER /* TODO: need to fix this for ARM */ # define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
I guess steven would like to see a patch which introduces both locked and lockless versions and with a very good comment explaining in what kind of unusual situation one can use the lockless version.
Thanks
Vivek
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 37 lines · Vivek Goyal
Quoted 36 lines · Huang Ying
Add __ftrace_enabled_save/restore, used to disable ftrace for a while. Now, this is used by kexec jump, which need a version without lock, for general situation, a locked version should be used.
Signed-off-by Huang Ying <[email protected]>--- include/linux/ftrace.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)include/linux/ftrace.h +20
-0@@ -98,6 +98,27 @@ static inline void tracer_disable(void)#endif } +/* Ftrace disable/restore without lock. Some synchronization mechanism +* must be used to prevent ftrace_enabled to be changed between +* disable/restore. */ +static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else +return 0 ; +#endif +} + +static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +}
Quoted 8 lines
include/linux/ftrace.h static inline void tracer_disable(void) · line 124
Quoted 4 lines
include/linux/ftrace.h +1
-0+#ifdef CONFIG_FRAME_POINTER /* TODO: need to fix this for ARM */ # define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))I guess steven would like to see a patch which introduces both locked and lockless versions and with a very good comment explaining in what kind of unusual situation one can use the lockless version.
Have sent a locked version to Steven. And, there are some comments for non-locked version, __ftrace_enabled_save() in above patch. What do you think about it?
Best Regards,
Huang Ying
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 3 lines · Huang Ying
+/* Ftrace disable/restore without lock. Some synchronization mechanism
+ * must be used to preventftrace_enabledto be changed between
+ * disable/restore. */
use the proper comment style please:
/*
*
*/
Quoted 7 lines
include/linux/ftrace.h +7
-0+static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else
include/linux/ftrace.h static inline void tracer_disable(void) · line 120
Quoted 10 lines
include/linux/ftrace.h +10
-0+return 0 ; +#endif +} + +static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +}
hm, what is this used for?
also, instead of such an ugly inline, why not create a proper kernel/trace/* function for this. That would also give it access to all the proper locking mechanisms - instead of relying on some extral mechanism.
Ingo
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 11 lines · Ingo Molnar
Quoted 3 lines · Huang Ying
+/* Ftrace disable/restore without lock. Some synchronization mechanism
+ * must be used to preventftrace_enabledto be changed between
+ * disable/restore. */use the proper comment style please:
/*
*
*/
OK. I will change it.
Quoted 20 lines
Quoted 7 lines
include/linux/ftrace.h +7
-0+static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else include/linux/ftrace.h static inline void tracer_disable(void) · line 120
Quoted 10 lines
include/linux/ftrace.h +10
-0+return 0 ; +#endif +} + +static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +}hm, what is this used for?
Quoted 4 lines
also, instead of such an ugly inline, why not create a proper
kernel/trace/*function for this. That would also give it access to all the proper locking mechanisms - instead of relying on some extral mechanism.
This function is used for kexec jump in machine_kexec(). Where all non-boot CPUs and IRQ are disabled, system is going to kexec, and it is not allowed to schedule to other process in this circumstance, so a non-lock version is needed. A locked version has been implemented by Steven Rostedt, I think it can be used for other circumstance.
Best Regards,
Huang Ying
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 7 lines · Huang Ying
Quoted 6 lines · Ingo Molnar
Quoted 3 lines · Huang Ying
+/* Ftrace disable/restore without lock. Some synchronization mechanism
+ * must be used to preventftrace_enabledto be changed between
+ * disable/restore. */
Quoted 7 lines
Quoted 5 lines
use the proper comment style please:
/*
*
*/OK. I will change it.
it's upstream already, please send a followup clean up patch.
Quoted 25 lines
Quoted 20 lines
Quoted 7 lines
include/linux/ftrace.h +7
-0+static inline int __ftrace_enabled_save (void ) +{ +#ifdef CONFIG_FTRACE +int saved_ftrace_enabled = ftrace_enabled; + ftrace_enabled =0 ; +return saved_ftrace_enabled; +#else include/linux/ftrace.h static inline void tracer_disable(void) · line 120
Quoted 10 lines
include/linux/ftrace.h +10
-0+return 0 ; +#endif +} + +static inline void __ftrace_enabled_restore (int enabled) +{ +#ifdef CONFIG_FTRACE + ftrace_enabled = enabled; +#endif +}hm, what is this used for?
Quoted 4 lines
also, instead of such an ugly inline, why not create a proper
kernel/trace/*function for this. That would also give it access to all the proper locking mechanisms - instead of relying on some extral mechanism.
Quoted 5 lines
This function is used for kexec jump in
machine_kexec(). Where all non-boot CPUs and IRQ are disabled, system is going to kexec, and it is not allowed to schedule to other process in this circumstance, so a non-lock version is needed. A locked version has been implemented by Steven Rostedt, I think it can be used for other circumstance.
ok, fair enough.
Ingo
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec