On Sun, 25 Oct 2026 01:25:42 +0900
"Masami Hiramatsu (Google)" <[email protected]> wrote:
> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> Cc: [email protected]
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> kernel/trace/trace_events_filter.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c
> b/kernel/trace/trace_events_filter.c
> index 18ce3ec00d34..c2c35b4de85e 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -2332,8 +2332,10 @@ static int process_system_preds(struct
> trace_subsystem_dir *dir,
> filter_disable(file);
> parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
> append_filter_err(tr, pe, filter);
> - } else
> + } else {
> event_set_filtered_flag(file);
> + fail = false;
> + }
>
>
> filter_item = kzalloc_obj(*filter_item);
> @@ -2348,8 +2350,6 @@ static int process_system_preds(struct
> trace_subsystem_dir *dir,
> filter_item->filter = event_filter(file);
> event_set_filter(file, filter);
> filter = NULL;
> -
> - fail = false;
> }
>
> if (fail)
> @@ -2364,14 +2364,17 @@ static int process_system_preds(struct
> trace_subsystem_dir *dir,
> return 0;
> fail:
> /* No call succeeded */
> - free_filter_list(filter_list);
> + if (!list_empty(&filter_list->list))
> + delay_free_filter(filter_list);
> + else
> + free_filter_list(filter_list);
> parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
> return -EINVAL;
> fail_mem:
> __free_filter(filter);
>
> - /* If any call succeeded, we still need to sync */
> - if (!fail)
> + /* If any filter was replaced, we still need to sync */
> + if (!list_empty(&filter_list->list))
> delay_free_filter(filter_list);
> else
> free_filter_list(filter_list);
Hmm, I think this can be cleaned up a bit better.
1) remove the fail boolean
2) consolidate the if (!list_empty()) logic.
Like this:
diff --git a/kernel/trace/trace_events_filter.c
b/kernel/trace/trace_events_filter.c
index 5e74d2dae40a..4d763c55bb7b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2305,7 +2305,7 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
struct filter_list *filter_item;
struct event_filter *filter = NULL;
struct filter_head *filter_list;
- bool fail = true;
+ int ret = -ENOMEM;
int err;
filter_list = kmalloc_obj(*filter_list);
@@ -2332,8 +2332,10 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
filter_disable(file);
parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
append_filter_err(tr, pe, filter);
- } else
+ } else {
event_set_filtered_flag(file);
+ ret = 0;
+ }
filter_item = kzalloc_obj(*filter_item);
@@ -2348,11 +2350,9 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
filter_item->filter = event_filter(file);
event_set_filter(file, filter);
filter = NULL;
-
- fail = false;
}
- if (fail)
+ if (ret)
goto fail;
/*
@@ -2364,19 +2364,18 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
return 0;
fail:
/* No call succeeded */
- free_filter_list(filter_list);
parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
- return -EINVAL;
+ ret = -EINVAL;
fail_mem:
__free_filter(filter);
- /* If any call succeeded, we still need to sync */
- if (!fail)
+ /* If any filter was replaced, we still need to sync */
+ if (!list_empty(&filter_list->list))
delay_free_filter(filter_list);
else
free_filter_list(filter_list);
- return -ENOMEM;
+ return ret;
}
static int create_filter_start(char *filter_string, bool set_str,
-- Steve
