Re: [PATCH] fgraph: Give ret_stack its own kmem cache

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 21 Oct 2024 08:58:11 +0100
Ryan Roberts <ryan.roberts@arm.com> wrote:

> > @@ -1290,6 +1305,16 @@ int register_ftrace_graph(struct fgraph_ops *gops)
> >  
> >  	mutex_lock(&ftrace_lock);
> >  
> > +	if (!fgraph_stack_cachep)
> > +		fgraph_stack_cachep = kmem_cache_create("fgraph_stack",
> > +							SHADOW_STACK_SIZE,
> > +							SHADOW_STACK_SIZE, 0, NULL);  
> 
> (I don't have any experience with this code, but...) is there any value/need to
> destroy the cache in unregister_ftrace_graph()? I guess you would need to
> refcount it, so its created on the first call to register and destroyed on the
> last call to unregister?

No, we can't destroy it. In fact, we can't destroy the stacks
themselves until the task exits. This is because a function could have
been traced and its return address gets replaced by the fgraph return
code. Then it goes to sleep. For example, say you were tracing poll,
and systemd did a poll and you traced it. Now it may be sleeping
forever, waiting for some input. When it finally wakes up and exits the
function, it will need to get its original return address back.

The ret_stack holds the original return address that is needed when the
function finishes. Thus, its not safe to free it even when tracing is
finished. The callbacks may not be called when tracing is done, but the
ret_stack used to do the tracing will be called long after tracing is
over.

Now I'm looking at being able to free stacks by scanning all the tasks
after tracing is over and if the stack isn't being used (it's easy to
know if it is or not) then we can free it. But for those cases where
they are still being used, then we just have to give up and leave it be.

-- Steve