[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221122165938.353ee7c0@gandalf.local.home>
Date: Tue, 22 Nov 2022 16:59:38 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Mark Rutland <mark.rutland@....com>
Cc: linux-kernel@...r.kernel.org, mhiramat@...nel.org,
revest@...omium.org
Subject: Re: [PATCH 2/3] ftrace: export ftrace_free_filter() to modules
On Thu, 3 Nov 2022 17:09:06 +0000
Mark Rutland <mark.rutland@....com> wrote:
FYI, the subject should start with a capital letter.
ftrace: Export ftrace_free_filter() to modules
> Setting filters on an ftrace ops results in some memory being allocated
> for the filter hashes, which must be freed before the ops can be freed.
> This can be done by removing every individual element of the hash by
> calling ftrace_set_filter_ip() or ftrace_set_filter_ips() with `remove`
> set, but this is somewhat error prone as it's easy to forget to remove
> an element.
>
> Make it easier to clean this up by exporting ftrace_free_filter(), which
> can be used to clean up all of the filter hashes after an ftrace_ops has
> been unregistered.
>
> Using this, fix the ftrace-direct* samples to free hashes prior to being
> unloaded. All other code either removes individual filters explicitly or
> is built-in and already calls ftrace_free_filter().
>
> Signed-off-by: Mark Rutland <mark.rutland@....com>
> Cc: Florent Revest <revest@...omium.org>
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> ---
> kernel/trace/ftrace.c | 23 ++++++++++++++++++++-
> samples/ftrace/ftrace-direct-multi-modify.c | 1 +
> samples/ftrace/ftrace-direct-multi.c | 1 +
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index fbf2543111c05..1ecdda1df6d47 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1248,12 +1248,17 @@ static void free_ftrace_hash_rcu(struct ftrace_hash *hash)
> call_rcu(&hash->rcu, __free_ftrace_hash_rcu);
> }
>
> +/**
> + * ftrace_free_filter - remove all filters for an ftrace_ops
> + * @ops - the ops to remove the filters from
> + */
> void ftrace_free_filter(struct ftrace_ops *ops)
> {
> ftrace_ops_init(ops);
> free_ftrace_hash(ops->func_hash->filter_hash);
> free_ftrace_hash(ops->func_hash->notrace_hash);
> }
> +EXPORT_SYMBOL_GPL(ftrace_free_filter);
>
> static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
> {
> @@ -5837,6 +5842,10 @@ EXPORT_SYMBOL_GPL(modify_ftrace_direct_multi);
> *
> * Filters denote which functions should be enabled when tracing is enabled
> * If @ip is NULL, it fails to update filter.
> + *
> + * This can allocate memory which must be freed before @ops can be freed,
> + * either by remvoing eached filtered addr or by using
> + * ftrace_free_filter(@ops).
> */
> int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
> int remove, int reset)
> @@ -5856,7 +5865,11 @@ EXPORT_SYMBOL_GPL(ftrace_set_filter_ip);
> *
> * Filters denote which functions should be enabled when tracing is enabled
> * If @ips array or any ip specified within is NULL , it fails to update filter.
> - */
> + *
> + * This can allocate memory which must be freed before @ops can be freed,
> + * either by remvoing eached filtered addr or by using
> + * ftrace_free_filter(@ops).
> +*/
> int ftrace_set_filter_ips(struct ftrace_ops *ops, unsigned long *ips,
> unsigned int cnt, int remove, int reset)
> {
> @@ -5898,6 +5911,10 @@ ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
> *
> * Filters denote which functions should be enabled when tracing is enabled.
> * If @buf is NULL and reset is set, all functions will be enabled for tracing.
> + *
> + * This can allocate memory which must be freed before @ops can be freed,
> + * either by remvoing eached filtered addr or by using
> + * ftrace_free_filter(@ops).
> */
> int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
> int len, int reset)
> @@ -5917,6 +5934,10 @@ EXPORT_SYMBOL_GPL(ftrace_set_filter);
> * Notrace Filters denote which functions should not be enabled when tracing
> * is enabled. If @buf is NULL and reset is set, all functions will be enabled
> * for tracing.
> + *
> + * This can allocate memory which must be freed before @ops can be freed,
> + * either by remvoing eached filtered addr or by using
I love how you cut and pasted two typos four times. ;-)
-- Steve
> + * ftrace_free_filter(@ops).
> */
> int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
> int len, int reset)
> diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
> index d52370cad0b6e..a825dbd2c9cfd 100644
> --- a/samples/ftrace/ftrace-direct-multi-modify.c
> +++ b/samples/ftrace/ftrace-direct-multi-modify.c
> @@ -152,6 +152,7 @@ static void __exit ftrace_direct_multi_exit(void)
> {
> kthread_stop(simple_tsk);
> unregister_ftrace_direct_multi(&direct, my_tramp);
> + ftrace_free_filter(&direct);
> }
>
> module_init(ftrace_direct_multi_init);
> diff --git a/samples/ftrace/ftrace-direct-multi.c b/samples/ftrace/ftrace-direct-multi.c
> index ec1088922517d..d955a26506053 100644
> --- a/samples/ftrace/ftrace-direct-multi.c
> +++ b/samples/ftrace/ftrace-direct-multi.c
> @@ -79,6 +79,7 @@ static int __init ftrace_direct_multi_init(void)
> static void __exit ftrace_direct_multi_exit(void)
> {
> unregister_ftrace_direct_multi(&direct, (unsigned long) my_tramp);
> + ftrace_free_filter(&direct);
> }
>
> module_init(ftrace_direct_multi_init);
Powered by blists - more mailing lists