2023-08-10 20:25:17

by Eric Vaughn

Subject: [PATCH] tracing/user_events: Optimize safe list traversals

Several of the list traversals in the user_events facility use safe list
traversals where they could be using the unsafe versions instead.

Replace these safe traversals with their unsafe counterparts in the
interest of optimization.

Suggested-by: Beau Belgrave <[email protected]>
Signed-off-by: Eric Vaughn <[email protected]>
---
kernel/trace/trace_events_user.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index 33cb6af31f39..6f046650e527 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -1328,14 +1328,14 @@ static int user_field_set_string(struct ftrace_event_field *field,

static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)
{
- struct ftrace_event_field *field, *next;
+ struct ftrace_event_field *field;
struct list_head *head = &user->fields;
int pos = 0, depth = 0;
const char *str_func;

pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");

- list_for_each_entry_safe_reverse(field, next, head, link) {
+ list_for_each_entry_reverse(field, head, link) {
if (depth != 0)
pos += snprintf(buf + pos, LEN_OR_ZERO, " ");

@@ -1347,7 +1347,7 @@ static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)

pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");

- list_for_each_entry_safe_reverse(field, next, head, link) {
+ list_for_each_entry_reverse(field, head, link) {
if (user_field_is_dyn_string(field->type, &str_func))
pos += snprintf(buf + pos, LEN_OR_ZERO,
", %s(%s)", str_func, field->name);
@@ -1732,7 +1732,7 @@ static int user_event_create(const char *raw_command)
static int user_event_show(struct seq_file *m, struct dyn_event *ev)
{
struct user_event *user = container_of(ev, struct user_event, devent);
- struct ftrace_event_field *field, *next;
+ struct ftrace_event_field *field;
struct list_head *head;
int depth = 0;

@@ -1740,7 +1740,7 @@ static int user_event_show(struct seq_file *m, struct dyn_event *ev)

head = trace_get_fields(&user->call);

- list_for_each_entry_safe_reverse(field, next, head, link) {
+ list_for_each_entry_reverse(field, head, link) {
if (depth == 0)
seq_puts(m, " ");
else
@@ -1816,13 +1816,14 @@ static bool user_field_match(struct ftrace_event_field *field, int argc,
static bool user_fields_match(struct user_event *user, int argc,
const char **argv)
{
- struct ftrace_event_field *field, *next;
+ struct ftrace_event_field *field;
struct list_head *head = &user->fields;
int i = 0;

- list_for_each_entry_safe_reverse(field, next, head, link)
+ list_for_each_entry_reverse(field, head, link) {
if (!user_field_match(field, argc, argv, &i))
return false;
+ }

if (i != argc)
return false;
--
2.34.1

2023-08-11 16:29:04

by Beau Belgrave

Subject: Re: [PATCH] tracing/user_events: Optimize safe list traversals

On Thu, Aug 10, 2023 at 12:43:37PM -0700, Eric Vaughn wrote:
> Several of the list traversals in the user_events facility use safe list
> traversals where they could be using the unsafe versions instead.
>
> Replace these safe traversals with their unsafe counterparts in the
> interest of optimization.
>
> Suggested-by: Beau Belgrave <[email protected]>
> Signed-off-by: Eric Vaughn <[email protected]>
> ---

Thanks for doing this, it looks good to me.

Acked-by: Beau Belgrave <[email protected]>

Thanks,
-Beau

> kernel/trace/trace_events_user.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 33cb6af31f39..6f046650e527 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -1328,14 +1328,14 @@ static int user_field_set_string(struct ftrace_event_field *field,
>
> static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)
> {
> - struct ftrace_event_field *field, *next;
> + struct ftrace_event_field *field;
> struct list_head *head = &user->fields;
> int pos = 0, depth = 0;
> const char *str_func;
>
> pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");
>
> - list_for_each_entry_safe_reverse(field, next, head, link) {
> + list_for_each_entry_reverse(field, head, link) {
> if (depth != 0)
> pos += snprintf(buf + pos, LEN_OR_ZERO, " ");
>
> @@ -1347,7 +1347,7 @@ static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)
>
> pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");
>
> - list_for_each_entry_safe_reverse(field, next, head, link) {
> + list_for_each_entry_reverse(field, head, link) {
> if (user_field_is_dyn_string(field->type, &str_func))
> pos += snprintf(buf + pos, LEN_OR_ZERO,
> ", %s(%s)", str_func, field->name);
> @@ -1732,7 +1732,7 @@ static int user_event_create(const char *raw_command)
> static int user_event_show(struct seq_file *m, struct dyn_event *ev)
> {
> struct user_event *user = container_of(ev, struct user_event, devent);
> - struct ftrace_event_field *field, *next;
> + struct ftrace_event_field *field;
> struct list_head *head;
> int depth = 0;
>
> @@ -1740,7 +1740,7 @@ static int user_event_show(struct seq_file *m, struct dyn_event *ev)
>
> head = trace_get_fields(&user->call);
>
> - list_for_each_entry_safe_reverse(field, next, head, link) {
> + list_for_each_entry_reverse(field, head, link) {
> if (depth == 0)
> seq_puts(m, " ");
> else
> @@ -1816,13 +1816,14 @@ static bool user_field_match(struct ftrace_event_field *field, int argc,
> static bool user_fields_match(struct user_event *user, int argc,
> const char **argv)
> {
> - struct ftrace_event_field *field, *next;
> + struct ftrace_event_field *field;
> struct list_head *head = &user->fields;
> int i = 0;
>
> - list_for_each_entry_safe_reverse(field, next, head, link)
> + list_for_each_entry_reverse(field, head, link) {
> if (!user_field_match(field, argc, argv, &i))
> return false;
> + }
>
> if (i != argc)
> return false;
> --
> 2.34.1