* [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
[not found] <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcas5p1.samsung.com>
@ 2017-12-12 11:54 ` Namit Gupta
[not found] ` <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcms5p1>
2017-12-28 16:32 ` Steven Rostedt
0 siblings, 2 replies; 7+ messages in thread
From: Namit Gupta @ 2017-12-12 11:54 UTC (permalink / raw)
To: rostedt; +Cc: mingo, linux-kernel, pankaj.m, a.sahrawat, Namit Gupta
ftrace_module_init happen after dynamic_debug_setup, it is desired that
cleanup should be called after this label however in current implementation
it is called in free module label,ie:even though ftrace in not initialized,
from so many fail case ftrace_release_mod() will be called and unnecessary
traverse the whole list.
In below patch we moved ftrace_release_mod() from free_module label to
ddebug_cleanup label. that is the best possible location, other solution
is to make new label to ftrace_release_mod() but since ftrace_module_init()
is not return with minimum changes it should be in ddebug_cleanup label.
Signed-off-by: Namit Gupta <gupta.namit@samsung.com>
---
kernel/module.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 0d1cb8d..4be966a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3523,6 +3523,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
unset_module_core_ro_nx(mod);
ddebug_cleanup:
+ ftrace_release_mod(mod);
dynamic_debug_remove(info->debug);
synchronize_sched();
kfree(mod->args);
@@ -3541,12 +3542,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
synchronize_rcu();
mutex_unlock(&module_mutex);
free_module:
- /*
- * Ftrace needs to clean up what it initialized.
- * This does nothing if ftrace_module_init() wasn't called,
- * but it must be called outside of module_mutex.
- */
- ftrace_release_mod(mod);
/* Free lock-classes; relies on the preceding sync_rcu() */
lockdep_free_key_range(mod->module_core, mod->core_size);
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
[not found] ` <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcms5p1>
@ 2017-12-28 5:14 ` NAMIT GUPTA
2017-12-28 16:28 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: NAMIT GUPTA @ 2017-12-28 5:14 UTC (permalink / raw)
To: rostedt; +Cc: mingo, linux-kernel, PANKAJ MISHRA, AMIT SAHRAWAT
Hello Steve,
As per the discussion I sent patch v2.
Please find.
Thanks,
Namit Gupta
--------- Original Message ---------
Sender : NAMIT GUPTA <gupta.namit@samsung.com> ./Senior Chief Engineer/SRI-Delhi-Platform S/W 1 Team/Samsung Electronics
Date : 2017-12-12 17:29 (GMT+5:30)
Title : [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
ftrace_module_init happen after dynamic_debug_setup, it is desired that
cleanup should be called after this label however in current implementation
it is called in free module label,ie:even though ftrace in not initialized,
from so many fail case ftrace_release_mod() will be called and unnecessary
traverse the whole list.
In below patch we moved ftrace_release_mod() from free_module label to
ddebug_cleanup label. that is the best possible location, other solution
is to make new label to ftrace_release_mod() but since ftrace_module_init()
is not return with minimum changes it should be in ddebug_cleanup label.
Signed-off-by: Namit Gupta <gupta.namit@samsung.com>
---
kernel/module.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 0d1cb8d..4be966a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3523,6 +3523,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
unset_module_core_ro_nx(mod);
ddebug_cleanup:
+ ftrace_release_mod(mod);
dynamic_debug_remove(info->debug);
synchronize_sched();
kfree(mod->args);
@@ -3541,12 +3542,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
synchronize_rcu();
mutex_unlock(&module_mutex);
free_module:
- /*
- * Ftrace needs to clean up what it initialized.
- * This does nothing if ftrace_module_init() wasn't called,
- * but it must be called outside of module_mutex.
- */
- ftrace_release_mod(mod);
/* Free lock-classes; relies on the preceding sync_rcu() */
lockdep_free_key_range(mod->module_core, mod->core_size);
--
1.9.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
2017-12-28 5:14 ` NAMIT GUPTA
@ 2017-12-28 16:28 ` Steven Rostedt
0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2017-12-28 16:28 UTC (permalink / raw)
To: NAMIT GUPTA; +Cc: mingo, linux-kernel, PANKAJ MISHRA, AMIT SAHRAWAT
On Thu, 28 Dec 2017 05:14:19 +0000
NAMIT GUPTA <gupta.namit@samsung.com> wrote:
>
> Hello Steve,
>
> As per the discussion I sent patch v2.
>
Thanks, I missed this due to traveling.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
2017-12-12 11:54 ` [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label Namit Gupta
[not found] ` <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcms5p1>
@ 2017-12-28 16:32 ` Steven Rostedt
2017-12-29 0:36 ` Jessica Yu
1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2017-12-28 16:32 UTC (permalink / raw)
To: Jessica Yu; +Cc: Namit Gupta, mingo, linux-kernel, pankaj.m, a.sahrawat, rusty
Jessica,
Can you take this patch. You can add:
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Thanks!
-- Steve
On Tue, 12 Dec 2017 17:24:32 +0530
Namit Gupta <gupta.namit@samsung.com> wrote:
> ftrace_module_init happen after dynamic_debug_setup, it is desired that
> cleanup should be called after this label however in current implementation
> it is called in free module label,ie:even though ftrace in not initialized,
> from so many fail case ftrace_release_mod() will be called and unnecessary
> traverse the whole list.
> In below patch we moved ftrace_release_mod() from free_module label to
> ddebug_cleanup label. that is the best possible location, other solution
> is to make new label to ftrace_release_mod() but since ftrace_module_init()
> is not return with minimum changes it should be in ddebug_cleanup label.
>
>
> Signed-off-by: Namit Gupta <gupta.namit@samsung.com>
>
> ---
> kernel/module.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 0d1cb8d..4be966a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3523,6 +3523,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> unset_module_core_ro_nx(mod);
>
> ddebug_cleanup:
> + ftrace_release_mod(mod);
> dynamic_debug_remove(info->debug);
> synchronize_sched();
> kfree(mod->args);
> @@ -3541,12 +3542,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
> synchronize_rcu();
> mutex_unlock(&module_mutex);
> free_module:
> - /*
> - * Ftrace needs to clean up what it initialized.
> - * This does nothing if ftrace_module_init() wasn't called,
> - * but it must be called outside of module_mutex.
> - */
> - ftrace_release_mod(mod);
> /* Free lock-classes; relies on the preceding sync_rcu() */
> lockdep_free_key_range(mod->module_core, mod->core_size);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
2017-12-28 16:32 ` Steven Rostedt
@ 2017-12-29 0:36 ` Jessica Yu
2017-12-29 0:57 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Jessica Yu @ 2017-12-29 0:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: Namit Gupta, mingo, linux-kernel, pankaj.m, a.sahrawat, rusty
+++ Steven Rostedt [28/12/17 11:32 -0500]:
>
>Jessica,
>
>Can you take this patch. You can add:
>
>Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Sure, thanks Steven.
Namit, your patch does not apply cleanly to modules-next nor
linux-next. It looks like you based your patch on an old tree?
(e.g., we got rid of the module_core and core_size fields a few years
ago...) Could you please rebase your patch on linux-next and resend?
Thanks,
Jessica
>On Tue, 12 Dec 2017 17:24:32 +0530
>Namit Gupta <gupta.namit@samsung.com> wrote:
>
>> ftrace_module_init happen after dynamic_debug_setup, it is desired that
>> cleanup should be called after this label however in current implementation
>> it is called in free module label,ie:even though ftrace in not initialized,
>> from so many fail case ftrace_release_mod() will be called and unnecessary
>> traverse the whole list.
>> In below patch we moved ftrace_release_mod() from free_module label to
>> ddebug_cleanup label. that is the best possible location, other solution
>> is to make new label to ftrace_release_mod() but since ftrace_module_init()
>> is not return with minimum changes it should be in ddebug_cleanup label.
>>
>>
>> Signed-off-by: Namit Gupta <gupta.namit@samsung.com>
>>
>> ---
>> kernel/module.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 0d1cb8d..4be966a 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3523,6 +3523,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>> unset_module_core_ro_nx(mod);
>>
>> ddebug_cleanup:
>> + ftrace_release_mod(mod);
>> dynamic_debug_remove(info->debug);
>> synchronize_sched();
>> kfree(mod->args);
>> @@ -3541,12 +3542,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>> synchronize_rcu();
>> mutex_unlock(&module_mutex);
>> free_module:
>> - /*
>> - * Ftrace needs to clean up what it initialized.
>> - * This does nothing if ftrace_module_init() wasn't called,
>> - * but it must be called outside of module_mutex.
>> - */
>> - ftrace_release_mod(mod);
>> /* Free lock-classes; relies on the preceding sync_rcu() */
>> lockdep_free_key_range(mod->module_core, mod->core_size);
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
2017-12-29 0:36 ` Jessica Yu
@ 2017-12-29 0:57 ` Steven Rostedt
2017-12-29 1:02 ` Jessica Yu
0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2017-12-29 0:57 UTC (permalink / raw)
To: Jessica Yu; +Cc: Namit Gupta, mingo, linux-kernel, pankaj.m, a.sahrawat, rusty
On Fri, 29 Dec 2017 01:36:48 +0100
Jessica Yu <jeyu@kernel.org> wrote:
> +++ Steven Rostedt [28/12/17 11:32 -0500]:
> >
> >Jessica,
> >
> >Can you take this patch. You can add:
> >
> >Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Sure, thanks Steven.
>
> Namit, your patch does not apply cleanly to modules-next nor
> linux-next. It looks like you based your patch on an old tree?
> (e.g., we got rid of the module_core and core_size fields a few years
> ago...) Could you please rebase your patch on linux-next and resend?
I should probably re-review the rebased patch.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ftrace/module: Move ftrace_release_mod to ddebug_cleanup label
2017-12-29 0:57 ` Steven Rostedt
@ 2017-12-29 1:02 ` Jessica Yu
0 siblings, 0 replies; 7+ messages in thread
From: Jessica Yu @ 2017-12-29 1:02 UTC (permalink / raw)
To: Steven Rostedt
Cc: Namit Gupta, mingo, linux-kernel, pankaj.m, a.sahrawat, rusty
+++ Steven Rostedt [28/12/17 19:57 -0500]:
>On Fri, 29 Dec 2017 01:36:48 +0100
>Jessica Yu <jeyu@kernel.org> wrote:
>
>> +++ Steven Rostedt [28/12/17 11:32 -0500]:
>> >
>> >Jessica,
>> >
>> >Can you take this patch. You can add:
>> >
>> >Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>
>> Sure, thanks Steven.
>>
>> Namit, your patch does not apply cleanly to modules-next nor
>> linux-next. It looks like you based your patch on an old tree?
>> (e.g., we got rid of the module_core and core_size fields a few years
>> ago...) Could you please rebase your patch on linux-next and resend?
>
>I should probably re-review the rebased patch.
Yeah, that'd be much appreciated :-)
Thanks!
Jessica
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-29 1:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcas5p1.samsung.com>
2017-12-12 11:54 ` [PATCH v2] ftrace/module: Move ftrace_release_mod to ddebug_cleanup label Namit Gupta
[not found] ` <CGME20171212115859epcas5p1c80cacda05c36d4ef2607df2450d4aa3@epcms5p1>
2017-12-28 5:14 ` NAMIT GUPTA
2017-12-28 16:28 ` Steven Rostedt
2017-12-28 16:32 ` Steven Rostedt
2017-12-29 0:36 ` Jessica Yu
2017-12-29 0:57 ` Steven Rostedt
2017-12-29 1:02 ` Jessica Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.