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

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Tue, 19 May 2026 08:31:28 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> > 
> > To access @data, simple casting does not work. Thus we need a
> > new syntax:
> > 
> > 	(STRUCT)(PTR,ASSIGN)->FIELD
> > 
> > So the above case, we can do:
> > 
> > 	data=(foo)(foo_list,list)->data
> 
> 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


> 
> 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.


> > > 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 :-)

> 
> > 
> > > +		id = bpf_find_btf_id(sname, BTF_KIND_STRUCT, &btf);
> > > +		if (id < 0)
> > > +			return -EINVAL;  
> > 
> > Why don't you return id (it has corresponding errno)?
> 
> Because I forgot to ;-)
> 
> > 
> > > +		ctx->btf = btf;
> > > +	} else {
> > > +		id = btf_find_by_name_kind(ctx->btf, sname, BTF_KIND_STRUCT);
> > > +		if (id < 0)
> > > +			return -EINVAL;  
> > 
> > Ditto.
> > 
> > > +	}
> > > +
> > > +	ctx->last_struct = btf_type_by_id(ctx->btf, id);
> > > +	return 0;
> > > +}
> > > +
> > >  static int query_btf_context(struct traceprobe_parse_context *ctx)
> > >  {
> > >  	const struct btf_param *param;
> > > @@ -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?

> 
> > 
> > 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.
 
Thank you!

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>