On Tue, 25 Jan 2022 21:11:46 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> From: Jiri Olsa <jolsa@xxxxxxxxxx>
>
> Adding ftrace_set_filter_ips function to be able to set filter on
> multiple ip addresses at once.
>
> With the kprobe multi attach interface we have cases where we need to
> initialize ftrace_ops object with thousands of functions, so having
> single function diving into ftrace_hash_move_and_update_ops with
> ftrace_lock is faster.
>
> The functions ips are passed as unsigned long array with count.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> include/linux/ftrace.h | 3 +++
> kernel/trace/ftrace.c | 53 ++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9999e29187de..60847cbce0da 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -512,6 +512,8 @@ struct dyn_ftrace {
>
> int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
> int remove, int reset);
> +int ftrace_set_filter_ips(struct ftrace_ops *ops, unsigned long *ips,
> + unsigned int cnt, int remove, int reset);
> int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
> int len, int reset);
> int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
> @@ -802,6 +804,7 @@ static inline unsigned long ftrace_location(unsigned long ip)
> #define ftrace_regex_open(ops, flag, inod, file) ({ -ENODEV; })
> #define ftrace_set_early_filter(ops, buf, enable) do { } while (0)
> #define ftrace_set_filter_ip(ops, ip, remove, reset) ({ -ENODEV; })
> +#define ftrace_set_filter_ips(ops, ips, cnt, remove, reset) ({ -ENODEV; })
> #define ftrace_set_filter(ops, buf, len, reset) ({ -ENODEV; })
> #define ftrace_set_notrace(ops, buf, len, reset) ({ -ENODEV; })
> #define ftrace_free_filter(ops) do { } while (0)
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index be5f6b32a012..39350aa38649 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4958,7 +4958,7 @@ ftrace_notrace_write(struct file *file, const char __user *ubuf,
> }
>
> static int
> -ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
> +__ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
> {
> struct ftrace_func_entry *entry;
>
> @@ -4976,9 +4976,25 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
> return add_hash_entry(hash, ip);
> }
>
> +static int
> +ftrace_match_addr(struct ftrace_hash *hash, unsigned long *ips,
> + unsigned int cnt, int remove)
> +{
> + unsigned int i;
> + int err;
> +
> + for (i = 0; i < cnt; i++) {
> + err = __ftrace_match_addr(hash, ips[i], remove);
> + if (err)
> + return err;
On error should we revert what was done?
goto err;
> + }
> + return 0;
err:
for (i--; i >= 0; i--)
__ftrace_match_addr(hash, ips[i], !remove);
return err;
Although it may not matter as it looks like it is only used on a temporary
hash. But either it should be commented that is the case, or we do the above
just to be more robust.
> +}
> +
> static int
> ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
> - unsigned long ip, int remove, int reset, int enable)
> + unsigned long *ips, unsigned int cnt,
> + int remove, int reset, int enable)
> {
> struct ftrace_hash **orig_hash;
> struct ftrace_hash *hash;
> @@ -5008,8 +5024,8 @@ ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
> ret = -EINVAL;
> goto out_regex_unlock;
> }
> - if (ip) {
> - ret = ftrace_match_addr(hash, ip, remove);
> + if (ips) {
> + ret = ftrace_match_addr(hash, ips, cnt, remove);
> if (ret < 0)
> goto out_regex_unlock;
> }
> @@ -5026,10 +5042,10 @@ ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
> }
>
> static int
> -ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
> - int reset, int enable)
> +ftrace_set_addr(struct ftrace_ops *ops, unsigned long *ips, unsigned int cnt,
> + int remove, int reset, int enable)
> {
> - return ftrace_set_hash(ops, NULL, 0, ip, remove, reset, enable);
> + return ftrace_set_hash(ops, NULL, 0, ips, cnt, remove, reset, enable);
> }
>
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> @@ -5634,10 +5650,29 @@ int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
> int remove, int reset)
> {
> ftrace_ops_init(ops);
> - return ftrace_set_addr(ops, ip, remove, reset, 1);
> + return ftrace_set_addr(ops, &ip, 1, remove, reset, 1);
> }
> EXPORT_SYMBOL_GPL(ftrace_set_filter_ip);
>
> +/**
> + * ftrace_set_filter_ips - set a functions to filter on in ftrace by addresses
- set functions to filter on ...
-- Steve
> + * @ops - the ops to set the filter with
> + * @ips - the array of addresses to add to or remove from the filter.
> + * @cnt - the number of addresses in @ips
> + * @remove - non zero to remove ips from the filter
> + * @reset - non zero to reset all filters before applying this filter.
> + *
> + * 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.
> + */
> +int ftrace_set_filter_ips(struct ftrace_ops *ops, unsigned long *ips,
> + unsigned int cnt, int remove, int reset)
> +{
> + ftrace_ops_init(ops);
> + return ftrace_set_addr(ops, ips, cnt, remove, reset, 1);
> +}
> +EXPORT_SYMBOL_GPL(ftrace_set_filter_ips);
> +
> /**
> * ftrace_ops_set_global_filter - setup ops to use global filters
> * @ops - the ops which will use the global filters
> @@ -5659,7 +5694,7 @@ static int
> ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
> int reset, int enable)
> {
> - return ftrace_set_hash(ops, buf, len, 0, 0, reset, enable);
> + return ftrace_set_hash(ops, buf, len, NULL, 0, 0, reset, enable);
> }
>
> /**
