FAILED: patch "[PATCH] ftrace: Protect ftrace_graph_hash with ftrace_sync" failed to apply to 5.5-stable tree

PATCH [email protected] 3 participants 4 messages Feb 7, 2020

Subsystems (3)

Stable branch Tracing ftrace

lore

Share & tools

lore

[email protected]

author maintainer

2020-02-07 10:16 UTC #0 lore raw

The patch below does not apply to the 5.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 54a16ff6f2e50775145b210bcd94d62c3c2af117 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <[email protected]>
Date: Wed, 5 Feb 2020 09:20:32 -0500
Subject: [PATCH] ftrace: Protect ftrace_graph_hash with ftrace_sync

As function_graph tracer can run when RCU is not "watching", it can not be protected by synchronize_rcu() it requires running a task on each CPU before it can be freed. Calling schedule_on_each_cpu(ftrace_sync) needs to be used.

Link https://lore.kernel.org/r/20200205131110.GT2935@paulmck-ThinkPad-P72 open here

Cc [email protected]

Fixes b9b0c831bed26 ("ftrace: Convert graph filter to use hash tables")

Reported-by "Paul E. McKenney" <[email protected]>

Reviewed-by Joel Fernandes (Google) <[email protected]>

Signed-off-by Steven Rostedt (VMware) <[email protected]>

kernel/trace/ftrace.c +9 -2

index 481ede3eac13..3f7ee102868a 100644
@@ -5867,8 +5867,15 @@ ftrace_graph_release(struct inode *inode, struct file *file)
 
 		mutex_unlock(&graph_lock);
 
-		/* Wait till all users are no longer using the old hash */
-		synchronize_rcu();
+		/*
+		 * We need to do a hard force of sched synchronization.
+		 * This is because we use preempt_disable() to do RCU, but
+		 * the function tracers can be called where RCU is not watching
+		 * (like before user_exit()). We can not rely on the RCU
+		 * infrastructure to do the synchronization, thus we must do it
+		 * ourselves.
+		 */
+		schedule_on_each_cpu(ftrace_sync);
 
 		free_ftrace_hash(old_hash);
 	}

kernel/trace/trace.h +2 -0

index 8c52f5de9384..3c75d29bd861 100644
@@ -979,6 +979,7 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	hash = rcu_dereference_protected(ftrace_graph_hash, !preemptible());

@@ -1031,6 +1032,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	notrace_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
 						 !preemptible());

Steven Rostedt

maintainer

2020-02-07 13:28 UTC #1 lore raw

Hi Greg,

Quoted 13 lines · On Fri, 07 Feb 2020 11:16:16 +0100 <[email protected]> wrote:

kernel/trace/trace.h +1 -0

index 8c52f5de9384..3c75d29bd861 100644
@@ -979,6 +979,7 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	hash = rcu_dereference_protected(ftrace_graph_hash, !preemptible());

kernel/trace/trace.h static inline int ftrace_graph_notrace_addr(unsigned long addr) · line 1038

Quoted 7 lines

kernel/trace/trace.h +1 -0

@@ -1031,6 +1032,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	notrace_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
 						 !preemptible());

Ah, I updated that patch to insert these comments, which makes it dependent on 16052dd5bdfa ("ftrace: Add comment to why rcu_dereference_sched() is open coded"). This is just adding comments and should have a very lower risk of breaking anything. If you add that patch first, then this patch should apply cleanly. Would it be OK to add that comment patch? It should fix most the conflicts.

-- Steve

Sasha Levin

maintainer

2020-02-07 15:07 UTC #2 lore raw

Quoted 18 lines · Steven Rostedt

Hi Greg,

Quoted 12 lines · On Fri, 07 Feb 2020 11:16:16 +0100 <[email protected]> wrote:

kernel/trace/trace.h +1 -0

index 8c52f5de9384..3c75d29bd861 100644
@@ -979,6 +979,7 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	hash = rcu_dereference_protected(ftrace_graph_hash, !preemptible());

Quoted 15 lines

kernel/trace/trace.h static inline int ftrace_graph_notrace_addr(unsigned long addr) · line 1038
Quoted 8 lines
kernel/trace/trace.h +1 -0
@@ -1031,6 +1032,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	notrace_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
 						 !preemptible());
Ah, I updated that patch to insert these comments, which makes it dependent on 16052dd5bdfa ("ftrace: Add comment to why rcu_dereference_sched() is open coded"). This is just adding comments and should have a very lower risk of breaking anything. If you add that patch first, then this patch should apply cleanly. Would it be OK to add that comment patch? It should fix most the conflicts.

I've ended up taking these additional commits, and queued everything for 5.5-4.14:

16052dd5bdfa ("ftrace: Add comment to why rcu_dereference_sched() is open coded") 24a9729f8314 ("tracing: Annotate ftrace_graph_hash pointer with __rcu") fd0e6852c407 ("tracing: Annotate ftrace_graph_notrace_hash pointer with __rcu")

Signature · 2 lines

Thanks,
Sasha

Steven Rostedt

maintainer

2020-02-07 16:53 UTC #3 lore raw

Quoted 6 lines · On Fri, 7 Feb 2020 10:07:28 -0500 Sasha Levin <[email protected]> wrote:

I've ended up taking these additional commits, and queued everything for 5.5-4.14:
16052dd5bdfa ("ftrace: Add comment to why rcu_dereference_sched() is open coded") 24a9729f8314 ("tracing: Annotate ftrace_graph_hash pointer with __rcu") fd0e6852c407 ("tracing: Annotate ftrace_graph_notrace_hash pointer with __rcu")

Thanks Sasha!

-- Steve