tracing: fprobe: Remove unused local variable

PATCH In review Masami Hiramatsu (Google) 2 participants 6 messages Sep 24, 2025 Tracing

lore

Share & tools

lore

Masami Hiramatsu (Google)

author maintainer

2025-09-24 00:26 UTC #0 lore raw

PATCH 1/2 tracing: fprobe: Remove unused local variable

From: Masami Hiramatsu (Google) <[email protected]>

The 'ret' local variable in fprobe_remove_node_in_module() was used for checking the error state in the loop, but commit dfe0d675df82 ("tracing: fprobe: use rhltable for fprobe_ip_table") removed the loop. So we don't need it anymore.

Fixes dfe0d675df82 ("tracing: fprobe: use rhltable for fprobe_ip_table")

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/fprobe.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

kernel/trace/fprobe.c +1 -4

index 6a205903b1ed..12ec194fdfed 100644
@@ -457,8 +457,6 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 static void fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist_node *node,
 					 struct fprobe_addr_list *alist)
 {
-	int ret = 0;
-
 	if (!within_module(node->addr, mod))
 		return;
 	if (delete_fprobe_node(node))

@@ -467,8 +465,7 @@ static void fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist
 	 * If failed to update alist, just continue to update hlist.
 	 * Therefore, at list user handler will not hit anymore.
 	 */
-	if (!ret)
-		ret = fprobe_addr_list_add(alist, node->addr);
+	fprobe_addr_list_add(alist, node->addr);
 }
 
 /* Handle module unloading to manage fprobe_ip_table. */

Masami Hiramatsu (Google)

author maintainer

2025-09-24 00:26 UTC #1 lore raw

PATCH 2/2 tracing: fprobe: Fix to remove recorded module addresses from filter

From: Masami Hiramatsu (Google) <[email protected]>

Even if there is a memory allocation failure in fprobe_addr_list_add(), there is a partial list of module addresses. So remove the recorded addresses from filter if exists.
This also removes the redundant ret local variable.

Fixes a3dc2983ca7b ("tracing: fprobe: Cleanup fprobe hash when module unloading")

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/fprobe.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

kernel/trace/fprobe.c +4 -3

index 12ec194fdfed..95e43814b85b 100644
@@ -434,8 +434,9 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 {
 	unsigned long *addrs;
 
-	if (alist->index >= alist->size)
-		return -ENOMEM;
+	/* Previously we failed to expand the list. */
+	if (alist->index == alist->size)
+		return -ENOSPC;
 
 	alist->addrs[alist->index++] = addr;
 	if (alist->index < alist->size)

@@ -497,7 +498,7 @@ static int fprobe_module_callback(struct notifier_block *nb,
 	} while (node == ERR_PTR(-EAGAIN));
 	rhashtable_walk_exit(&iter);
 
-	if (alist.index < alist.size && alist.index > 0)
+	if (alist.index > 0)
 		ftrace_set_filter_ips(&fprobe_graph_ops.ops,
 				      alist.addrs, alist.index, 1, 0);
 	mutex_unlock(&fprobe_mutex);

[email protected]

Reviewed-by Menglong Dong on v1 2/2

2025-09-24 01:55 UTC #2 lore raw

Re: [PATCH 2/2] tracing: fprobe: Fix to remove recorded module addresses from filter

On 2025/9/24 08:26 Masami Hiramatsu (Google) <[email protected]> write:

Quoted 6 lines

From: Masami Hiramatsu (Google) <[email protected]>
Even if there is a memory allocation failure in fprobe_addr_list_add(), there is a partial list of module addresses. So remove the recorded addresses from filter if exists.
This also removes the redundant ret local variable.

I think this is what you do in the previous patch?

The rest looks good to me.

Reviewed-by Menglong Dong <[email protected]>

(I think I should use Reviewed-by here, rather than Acked-by, right?)

Thanks!
Menglong Dong

Quoted 35 lines

Fixes a3dc2983ca7b ("tracing: fprobe: Cleanup fprobe hash when module unloading")

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/fprobe.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

kernel/trace/fprobe.c +4 -3

index 12ec194fdfed..95e43814b85b 100644
@@ -434,8 +434,9 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 {
 	unsigned long *addrs;
 
-	if (alist->index >= alist->size)
-		return -ENOMEM;
+	/* Previously we failed to expand the list. */
+	if (alist->index == alist->size)
+		return -ENOSPC;
 
 	alist->addrs[alist->index++] = addr;
 	if (alist->index < alist->size)

@@ -497,7 +498,7 @@ static int fprobe_module_callback(struct notifier_block *nb,
 	} while (node == ERR_PTR(-EAGAIN));
 	rhashtable_walk_exit(&iter);
 
-	if (alist.index < alist.size && alist.index > 0)
+	if (alist.index > 0)
 		ftrace_set_filter_ips(&fprobe_graph_ops.ops,
 				      alist.addrs, alist.index, 1, 0);
 	mutex_unlock(&fprobe_mutex);

[email protected]

2025-09-24 03:49 UTC #3 lore raw

Re: [PATCH 2/2] tracing: fprobe: Fix to remove recorded module addresses from filter

In reply to Masami Hiramatsu (Google)

On 2025/9/24 08:26 Masami Hiramatsu (Google) <[email protected]> write:

Quoted 7 lines

From: Masami Hiramatsu (Google) <[email protected]>
Even if there is a memory allocation failure in fprobe_addr_list_add(), there is a partial list of module addresses. So remove the recorded addresses from filter if exists.
This also removes the redundant ret local variable.

Quoted 5 lines

Fixes a3dc2983ca7b ("tracing: fprobe: Cleanup fprobe hash when module unloading")
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/fprobe.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Hi, Masami. Should I send the V2 of the patch:

tracing: fprobe: optimization for entry only case

after this series applied?

Thanks!
Menglong Dong

Quoted 29 lines

kernel/trace/fprobe.c +4 -3

index 12ec194fdfed..95e43814b85b 100644
@@ -434,8 +434,9 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 {
 	unsigned long *addrs;
 
-	if (alist->index >= alist->size)
-		return -ENOMEM;
+	/* Previously we failed to expand the list. */
+	if (alist->index == alist->size)
+		return -ENOSPC;
 
 	alist->addrs[alist->index++] = addr;
 	if (alist->index < alist->size)

@@ -497,7 +498,7 @@ static int fprobe_module_callback(struct notifier_block *nb,
 	} while (node == ERR_PTR(-EAGAIN));
 	rhashtable_walk_exit(&iter);
 
-	if (alist.index < alist.size && alist.index > 0)
+	if (alist.index > 0)
 		ftrace_set_filter_ips(&fprobe_graph_ops.ops,
 				      alist.addrs, alist.index, 1, 0);
 	mutex_unlock(&fprobe_mutex);

Masami Hiramatsu (Google)

author maintainer

2025-09-24 08:11 UTC #4 lore raw

Re: [PATCH 2/2] tracing: fprobe: Fix to remove recorded module addresses from filter

Quoted 14 lines · On Wed, 24 Sep 2025 11:49:39 +0800 [email protected] wrote:

On 2025/9/24 08:26 Masami Hiramatsu (Google) <[email protected]> write:
Quoted 7 lines
From: Masami Hiramatsu (Google) <[email protected]>
Even if there is a memory allocation failure in fprobe_addr_list_add(), there is a partial list of module addresses. So remove the recorded addresses from filter if exists.
This also removes the redundant ret local variable.
Quoted 5 lines
Fixes a3dc2983ca7b ("tracing: fprobe: Cleanup fprobe hash when module unloading")
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/fprobe.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Quoted 5 lines

Hi, Masami. Should I send the V2 of the patch:
tracing: fprobe: optimization for entry only case
after this series applied?

Yeah, I'll push this [2/2] to stable tonight. and push [1/2] to probes/for-next too. I'll rebase probes/for-next so that it will have both patch.

Thank you,

Quoted 37 lines

Thanks!
Menglong Dong

Quoted 29 lines

kernel/trace/fprobe.c +4 -3

index 12ec194fdfed..95e43814b85b 100644
@@ -434,8 +434,9 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 {
 	unsigned long *addrs;
 
-	if (alist->index >= alist->size)
-		return -ENOMEM;
+	/* Previously we failed to expand the list. */
+	if (alist->index == alist->size)
+		return -ENOSPC;
 
 	alist->addrs[alist->index++] = addr;
 	if (alist->index < alist->size)

@@ -497,7 +498,7 @@ static int fprobe_module_callback(struct notifier_block *nb,
 	} while (node == ERR_PTR(-EAGAIN));
 	rhashtable_walk_exit(&iter);
 
-	if (alist.index < alist.size && alist.index > 0)
+	if (alist.index > 0)
 		ftrace_set_filter_ips(&fprobe_graph_ops.ops,
 				      alist.addrs, alist.index, 1, 0);
 	mutex_unlock(&fprobe_mutex);

Signature · 1 line

Masami Hiramatsu (Google) <[email protected]>

[email protected]

Acked-by Menglong Dong on v1 1/2

2025-09-24 01:22 UTC #5 lore raw

In reply to Masami Hiramatsu (Google)

On 2025/9/24 08:26 Masami Hiramatsu (Google) <[email protected]> write:

Quoted 18 lines

From: Masami Hiramatsu (Google) <[email protected]>
The 'ret' local variable in fprobe_remove_node_in_module() was used for checking the error state in the loop, but commit dfe0d675df82 ("tracing: fprobe: use rhltable for fprobe_ip_table") removed the loop. So we don't need it anymore.
Fixes dfe0d675df82 ("tracing: fprobe: use rhltable for fprobe_ip_table")
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/fprobe.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
kernel/trace/fprobe.c +0 -0
index 6a205903b1ed..12ec194fdfed 100644

kernel/trace/fprobe.c static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad · line 460

Quoted 4 lines

kernel/trace/fprobe.c +0 -1

@@ -457,8 +457,6 @@ static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long ad
 static void fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist_node *node,
 					 struct fprobe_addr_list *alist)
 {
-	int ret = 0;

Acked-by Menglong Dong <[email protected]>

Thanks~

Quoted 17 lines

kernel/trace/fprobe.c +1 -3

-
 	if (!within_module(node->addr, mod))
 		return;
 	if (delete_fprobe_node(node))

@@ -467,8 +465,7 @@ static void fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist
 	 * If failed to update alist, just continue to update hlist.
 	 * Therefore, at list user handler will not hit anymore.
 	 */
-	if (!ret)
-		ret = fprobe_addr_list_add(alist, node->addr);
+	fprobe_addr_list_add(alist, node->addr);
 }
 
 /* Handle module unloading to manage fprobe_ip_table. */