On Mon, Jul 09, 2018 at 05:48:54PM -0400, Steven Rostedt wrote: > From: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx> > > It is unwise to take spin locks from the handlers of trace events. > Mainly, because they can introduce lockups, because it introduces locks > in places that are normally not tested. Worse yet, because trace events > are tucked away in the include/trace/events/ directory, locks that are > taken there are forgotten about. > > As a general rule, I tell people never to take any locks in a trace > event handler. > > Several cgroup trace event handlers call cgroup_path() which eventually > takes the kernfs_rename_lock spinlock. This injects the spinlock in the > code without people realizing it. It also can cause issues for the > PREEMPT_RT patch, as the spinlock becomes a mutex, and the trace event > handlers are called with preemption disabled. > > By moving the calculation of the cgroup_path() out of the trace event > handlers and into a macro (surrounded by a > trace_cgroup_##type##_enabled()), then we could place the cgroup_path > into a string, and pass that to the trace event. Not only does this > remove the taking of the spinlock out of the trace event handler, but > it also means that the cgroup_path() only needs to be called once (it > is currently called twice, once to get the length to reserver the > buffer for, and once again to get the path itself. Now it only needs to > be done once. > > Reported-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx> Applied to cgroup/for-4.19. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
