On Thu, 15 Aug 2024 14:37:18 +0200 Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > I checked bit more and there are more tracepoints with the same issue, > > the first diff stat looks like: > > > > include/trace/events/afs.h | 44 ++++++++++++++++++++++---------------------- > > include/trace/events/cachefiles.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------ > > include/trace/events/ext4.h | 6 +++--- > > include/trace/events/fib.h | 16 ++++++++-------- > > include/trace/events/filelock.h | 38 +++++++++++++++++++------------------- > > include/trace/events/host1x.h | 10 +++++----- > > include/trace/events/huge_memory.h | 24 ++++++++++++------------ > > include/trace/events/kmem.h | 18 +++++++++--------- > > include/trace/events/netfs.h | 16 ++++++++-------- > > include/trace/events/power.h | 6 +++--- > > include/trace/events/qdisc.h | 8 ++++---- > > include/trace/events/rxrpc.h | 12 ++++++------ > > include/trace/events/sched.h | 12 ++++++------ > > include/trace/events/sunrpc.h | 8 ++++---- > > include/trace/events/tcp.h | 14 +++++++------- > > include/trace/events/tegra_apb_dma.h | 6 +++--- > > include/trace/events/timer_migration.h | 10 +++++----- > > include/trace/events/writeback.h | 16 ++++++++-------- > > > > plus there's one case where pointer needs to be checked with IS_ERR in > > include/trace/events/rdma_core.h trace_mr_alloc/mr_integ_alloc > > > > I'm not excited about the '_nullable' argument suffix, because it's lot > > of extra changes/renames in TP_fast_assign and it does not solve the > > IS_ERR case above > > > > I checked on the type tag and with llvm build we get the TYPE_TAG info > > nicely in BTF: > > > > [119148] TYPEDEF 'btf_trace_sched_pi_setprio' type_id=119149 > > [119149] PTR '(anon)' type_id=119150 > > [119150] FUNC_PROTO '(anon)' ret_type_id=0 vlen=3 > > '(anon)' type_id=27 > > '(anon)' type_id=678 > > '(anon)' type_id=119152 > > [119151] TYPE_TAG 'nullable' type_id=679 > > [119152] PTR '(anon)' type_id=119151 > > > > [679] STRUCT 'task_struct' size=15424 vlen=277 > > > > which we can easily check in verifier.. the tracepoint definition would look like: > > > > - TP_PROTO(struct task_struct *tsk, struct task_struct *pi_task), > > + TP_PROTO(struct task_struct *tsk, struct task_struct __nullable *pi_task), > > > > and no other change in TP_fast_assign is needed > > > > I think using the type tag for this is nicer, but I'm not sure where's > > gcc at with btf_type_tag implementation, need to check on that > > Unfortunately last time I heard gcc was still far. > So we cannot rely on decl_tag or type_tag yet. > Aside from __nullable we would need another suffix to indicate is_err. > > Maybe we can do something with the TP* macro? > So the suffix only seen one place instead of search-and-replace > through the body? > > but imo above diff stat doesn't look too bad. I'm fine with annotation of parameters, but I really don't want this being part of the TP_fast_assign() content. What would that look like anyway? -- Steve
