Re: [PATCH v4] tracing/probes: Allow use of BTF names to dereference pointers

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 20 May 2026 00:26:40 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > Hmm, it may be better to make it one parenthesis?
> > 
> >        (STRUCT,PTR,ASSIGN)->FIELD
> > 
> >        data=(foo,foo_list,list)->data  
> 
> OK, but I don't like this 3 parameters conversion. I want to
> make it a kind of type casting with an option.
> 
> 	(STRUCT,ASSIGN)PTR->FIELD
> 
> 	data=(foo,list)foo_list->data
> 
> The second parenthesis will be eventually needed for nested casting,
> for example, in above case, if the data is a pointer to another data
> structure:
> 
> struct bar {
> 	int	value;
> 	...
> };
> 
> 	value=(bar)((foo,list)foo_list->data)->value

Have fun with the parenthesis parsing ;-)

> 
> 
> > 
> > That would make it easier to differentiate between a simple "typecast" and
> > a container_of() by checking if the content between the parenthesis has a
> > comma.
> > 
> > Maybe even reorder it to:
> > 
> >        (PTR,STRUCT,ASSIGN)->FIELD
> > 
> >        data=(foo_list,foo,list)->data
> > 
> > to match the order of container_of():
> > 
> >       data = container_of(foo_list, struct foo, list)->data;
> > 
> > ?  
> 
> This doesn't seem to conform to the rule here of using parentheses for
> type casting, so I personally don't like it.

OK, as long as it's intuitive and is easy to remember. I hate having to
look up the documents every time I have to do some probe argument
parsing :-(

> 
> 
> > > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > > index e0d3a0da26af..b0829eb1cb52 100644
> > > > --- a/kernel/trace/trace_probe.c
> > > > +++ b/kernel/trace/trace_probe.c
> > > > @@ -464,6 +464,26 @@ static const char *fetch_type_from_btf_type(struct btf *btf,
> > > >  	return NULL;
> > > >  }
> > > >  
> > > > +static int query_btf_struct(const char *sname, struct traceprobe_parse_context *ctx)
> > > > +{
> > > > +	int id;
> > > > +
> > > > +	if (!ctx->btf) {
> > > > +		struct btf *btf;    
> > > 
> > > This needs an empty line here.  
> > 
> > Sure.
> > 
> > For conditional blocks, I don't always add a newline, but this is your code
> > and I'll follow your suggestions.  
> 
> Ah, this is just for fixing checkpatch.pl warning :-)

I added it to keep your checkpatch happy.


> > > > @@ -471,12 +491,12 @@ static int query_btf_context(struct traceprobe_parse_context *ctx)
> > > >  	struct btf *btf;
> > > >  	s32 nr;
> > > >  
> > > > -	if (ctx->btf)
> > > > -		return 0;
> > > > -
> > > >  	if (!ctx->funcname)
> > > >  		return -EINVAL;
> > > >  
> > > > +	if (ctx->btf)
> > > > +		return 0;
> > > > +    
> > > 
> > > Could you tell me why this order is changed?
> > > I think this type casting will allow us to skip checking funcname
> > > because btf context is already specified.  
> > 
> > I wanted this to fail if btf was already set but funcname wasn't, because
> > this should only be called for functions.  
> 
> Hmm, OK. Then, can you make a separate patch for this?

I added a struct_btf (I can change it to typecast_btf) and have a
helper function that is:

static struct btf *ctx_btf(struct traceprobe_parse_context *ctx)
{
	return ctx->flags & TPARG_FL_STRUCT ?
		ctx->struct_btf : ctx->btf;
}

And have all functions get their btf from that. This way we can keep
the two allocations separate.

> 
> >   
> > > 
> > > Ah, BTW, we may need to use a special struct btf* for type
> > > casting. If the target function is in a module and the
> > > casting type is defined in vmlinux, those are stored in
> > > the different places...  
> > 
> > OK, I'll make a separate btf for it then. I'll have to make sure the btf
> > used for parsing knows which one to use. Shouldn't be too hard if we check
> > for the STRUCT flag in the ctx->flags.  
> 
> Yeah, and personally, I think that flag should be the TYPECAST flag.

I'll update it.

Thanks,

-- Steve