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

[Ryan Roberts <ryan.roberts@arm.com>]

On 21/10/2024 09:37, Steven Rostedt wrote:
> 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.

Ah, gotya. Thanks for the explanation!

> 
> -- Steve